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: require at least ARMv6 to enable the ARM DSP code #2169

Closed
wants to merge 1 commit into from
Closed

bn_mul.h: require at least ARMv6 to enable the ARM DSP code #2169

wants to merge 1 commit into from

Conversation

aurel32
Copy link
Contributor

@aurel32 aurel32 commented Nov 3, 2018

Description

Commit 16b1bd8 "bn_mul.h: add ARM DSP optimized MULADDC code"
added some ARM DSP instructions that was assumed to always be available
when __ARM_FEATURE_DSP is defined to 1. Unfortunately it appears that
the ARMv5TE architecture (GCC flag -march=armv5te) supports the DSP
instructions, but only in Thumb mode and not in ARM mode, despite
defining __ARM_FEATURE_DSP in both cases.

This patch fixes the build issue by requiring at least ARMv6 in addition
to the DSP feature.

Status

READY

Requires Backporting

NO, this is a bug fix for the development branch only, introduced by PR #1618.

Steps to test or reproduce

Build mbedTLS with -march=armv5te -marm

@aurel32
bn_mul.h: require at least ARMv6 to enable the ARM DSP code
63f5b85 
Commit 16b1bd8 "bn_mul.h: add ARM DSP optimized MULADDC code"
added some ARM DSP instructions that was assumed to always be available
when __ARM_FEATURE_DSP is defined to 1. Unfortunately it appears that
the ARMv5TE architecture (GCC flag -march=armv5te) supports the DSP
instructions, but only in Thumb mode and not in ARM mode, despite
defining __ARM_FEATURE_DSP in both cases.

This patch fixes the build issue by requiring at least ARMv6 in addition
to the DSP feature.
Patater
Patater approved these changes Nov 3, 2018
View reviewed changes
Copy link
Contributor

@Patater Patater left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@RonEld RonEld added the component-crypto Crypto primitives and low-level interfaces label Feb 18, 2019
@jcowgill
Copy link
Contributor

Please can this be merged. I currently have to carry this patch in the Debian package because mbedTLS won't build on armel without it.

@hanno-becker hanno-becker added the needs-ci Needs to pass CI tests label Jul 16, 2019
@Patater Patater added approved Design and code approved - may be waiting for CI or backports and removed needs-ci Needs to pass CI tests labels Jul 16, 2019
@gilles-peskine-arm
Copy link
Contributor

Unfortunately, the has been released in Mbed TLS 2.14. So this needs a changelog entry and a backport to Mbed TLS 2.16. I'll make those.

@gilles-peskine-arm gilles-peskine-arm added bug needs-work needs-backports Backports are missing or are pending review and approval. and removed approved Design and code approved - may be waiting for CI or backports labels Aug 2, 2019
This was referenced Aug 5, 2019
Merged
Merged
@gilles-peskine-arm
Copy link
Contributor

I made separate PRs for development and mbedtls-2.16 to facilitate testing: #2777, #2778. So this PR is now superseded.

@gilles-peskine-arm
Copy link
Contributor

@jcowgill It's cheap for us to add a build to our CI if it uses tools that are already available in our CI environment, but more work to add a new tool. We have arm-none-eabi-gcc but not arm-linux-*. I added a build with -march=armv5te, but I can tweak that. Would you suggest different flags to try not to break Debian?

@jcowgill
Copy link
Contributor

jcowgill commented Aug 5, 2019

I think -march=armv5te matches Debian's armel port, so just using that option will be fine.

@gilles-peskine-arm
Copy link
Contributor

@jcowgill The fix to bn_mul.h has been merged to Mbed TLS's development branch, as well as to the mbedtls-2.16 LTS branch. Thank you for your patience and cooperation!

kraj pushed a commit to YoeDistro/meta-openembedded that referenced this pull request Nov 13, 2019
@denix0 @kraj
mbedtls: upgrade 2.16.2 -> 2.16.3
7447eed 
* Mbed TLS 2.16.3 is a maintenance release of the Mbed TLS 2.16 branch, and
  provides bug fixes and minor enhancements.

https://github.com/ARMmbed/mbedtls/releases/tag/mbedtls-2.16.3

Most importantly, this fixes breakage on ARMv5TE platforms:

* Fix the build on ARMv5TE in ARM mode to not use assembly instructions that
  are only available in Thumb mode.

Mbed-TLS/mbedtls#2169

Signed-off-by: Denys Dmytriyenko <denys@ti.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
kraj pushed a commit to YoeDistro/meta-openembedded that referenced this pull request Nov 13, 2019
@denix0 @kraj
mbedtls: upgrade 2.16.2 -> 2.16.3
865ecb2 
* Mbed TLS 2.16.3 is a maintenance release of the Mbed TLS 2.16 branch, and
  provides bug fixes and minor enhancements.

https://github.com/ARMmbed/mbedtls/releases/tag/mbedtls-2.16.3

Most importantly, this fixes breakage on ARMv5TE platforms:

* Fix the build on ARMv5TE in ARM mode to not use assembly instructions that
  are only available in Thumb mode.

Mbed-TLS/mbedtls#2169

Signed-off-by: Denys Dmytriyenko <denys@ti.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
jpuhlman pushed a commit to MontaVista-OpenSourceTechnology/meta-openembedded that referenced this pull request Nov 14, 2019
@denix0 @jpuhlman
mbedtls: upgrade 2.16.2 -> 2.16.3
afee14d 
Source: meta-openembedded
MR: 00000
Type: Integration
Disposition: Merged from meta-openembedded
ChangeID: 865ecb2
Description:

* Mbed TLS 2.16.3 is a maintenance release of the Mbed TLS 2.16 branch, and
  provides bug fixes and minor enhancements.

https://github.com/ARMmbed/mbedtls/releases/tag/mbedtls-2.16.3

Most importantly, this fixes breakage on ARMv5TE platforms:

* Fix the build on ARMv5TE in ARM mode to not use assembly instructions that
  are only available in Thumb mode.

Mbed-TLS/mbedtls#2169

Signed-off-by: Denys Dmytriyenko <denys@ti.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
Signed-off-by: Jeremy Puhlman <jpuhlman@mvista.com>
halstead pushed a commit to openembedded/meta-openembedded that referenced this pull request Dec 8, 2019
@denix0 @akuster
mbedtls: upgrade 2.16.2 -> 2.16.3
0d16b31 
* Mbed TLS 2.16.3 is a maintenance release of the Mbed TLS 2.16 branch, and
  provides bug fixes and minor enhancements.

https://github.com/ARMmbed/mbedtls/releases/tag/mbedtls-2.16.3

Most importantly, this fixes breakage on ARMv5TE platforms:

* Fix the build on ARMv5TE in ARM mode to not use assembly instructions that
  are only available in Thumb mode.

Mbed-TLS/mbedtls#2169

Signed-off-by: Denys Dmytriyenko <denys@ti.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
Signed-off-by: Armin Kuster <akuster808@gmail.com>
jpuhlman pushed a commit to MontaVista-OpenSourceTechnology/meta-openembedded that referenced this pull request Dec 10, 2019
@denix0 @jpuhlman
mbedtls: upgrade 2.16.2 -> 2.16.3
2c63fe1 
Source: meta-openembedded
MR: 00000
Type: Integration
Disposition: Merged from meta-openembedded
ChangeID: 0d16b31
Description:

* Mbed TLS 2.16.3 is a maintenance release of the Mbed TLS 2.16 branch, and
  provides bug fixes and minor enhancements.

https://github.com/ARMmbed/mbedtls/releases/tag/mbedtls-2.16.3

Most importantly, this fixes breakage on ARMv5TE platforms:

* Fix the build on ARMv5TE in ARM mode to not use assembly instructions that
  are only available in Thumb mode.

Mbed-TLS/mbedtls#2169

Signed-off-by: Denys Dmytriyenko <denys@ti.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
Signed-off-by: Armin Kuster <akuster808@gmail.com>
Signed-off-by: Jeremy Puhlman <jpuhlman@mvista.com>
@gilles-peskine-arm gilles-peskine-arm mentioned this pull request May 4, 2020
Open
@rettichschnidi rettichschnidi mentioned this pull request Jun 13, 2020
Closed
daregit pushed a commit to daregit/yocto-combined that referenced this pull request May 22, 2024
@denix0 @kraj
src/meta-openembedded:mbedtls: upgrade 2.16.2 -> 2.16.3
6145bfc 
* Mbed TLS 2.16.3 is a maintenance release of the Mbed TLS 2.16 branch, and
  provides bug fixes and minor enhancements.

https://github.com/ARMmbed/mbedtls/releases/tag/mbedtls-2.16.3

Most importantly, this fixes breakage on ARMv5TE platforms:

* Fix the build on ARMv5TE in ARM mode to not use assembly instructions that
  are only available in Thumb mode.

Mbed-TLS/mbedtls#2169

Signed-off-by: Denys Dmytriyenko <denys@ti.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
daregit pushed a commit to daregit/yocto-combined that referenced this pull request May 22, 2024
@denix0 @kraj
src/meta-openembedded:mbedtls: upgrade 2.16.2 -> 2.16.3
3e233e8 
* Mbed TLS 2.16.3 is a maintenance release of the Mbed TLS 2.16 branch, and
  provides bug fixes and minor enhancements.

https://github.com/ARMmbed/mbedtls/releases/tag/mbedtls-2.16.3

Most importantly, this fixes breakage on ARMv5TE platforms:

* Fix the build on ARMv5TE in ARM mode to not use assembly instructions that
  are only available in Thumb mode.

Mbed-TLS/mbedtls#2169

Signed-off-by: Denys Dmytriyenko <denys@ti.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
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 approved these changes

@Patater Patater Patater 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-work
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@aurel32 @jcowgill @gilles-peskine-arm @Patater @hanno-becker @RonEld