-
Notifications
You must be signed in to change notification settings - Fork 140
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
Fixes for some issues on Arm platforms #818
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The change in 73b0de0 has caused a number of real world regressions on arm32/armhfp/aarch32 users, in particular when running on ARMv8 based hardware. There's a number of cases where software can't be installed or the OS can't even be installed due to issues around armv7hcnl and armv8hcnl. The NEON 'n' extension on ARMv8 hardware is not optional, so is assumed as part of the armv8hl rpm. The crypto extensions 'c' are optional and their implementation is widely varied, due to this the software implementations do run time detection, the detection of this functionality and subsquent priortisation of the 'c' extension is incorrect, especially where the software could be running in a VM or container and hence even in a running instance the underlying hardware could concievably change so requiring this as a compile time option has proved to be extremely problematic causing massive issues with Fedora ARM. We adjust the detection of NEON 'n' just to happen on ARMv7 where while it was implemented it has always been problematic and never really used and hence is a legacy implementation detail that needs to be supported but in reality the vast majority of NEON optimisation happens at run time in libraries where it make a difference. In the case of the 'c' crypto extenions we have explicitly not added this as package management functionality as it's handled effectively with run time detection as optimisaiton and as in Fefora has just caused regressions and issues. Fixes RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1691430 Signed-off-by: Peter Robinson <pbrobinson@gmail.com>
@rh-atomic-bot try |
rh-atomic-bot
pushed a commit
that referenced
this pull request
Oct 18, 2019
The change in 73b0de0 has caused a number of real world regressions on arm32/armhfp/aarch32 users, in particular when running on ARMv8 based hardware. There's a number of cases where software can't be installed or the OS can't even be installed due to issues around armv7hcnl and armv8hcnl. The NEON 'n' extension on ARMv8 hardware is not optional, so is assumed as part of the armv8hl rpm. The crypto extensions 'c' are optional and their implementation is widely varied, due to this the software implementations do run time detection, the detection of this functionality and subsquent priortisation of the 'c' extension is incorrect, especially where the software could be running in a VM or container and hence even in a running instance the underlying hardware could concievably change so requiring this as a compile time option has proved to be extremely problematic causing massive issues with Fedora ARM. We adjust the detection of NEON 'n' just to happen on ARMv7 where while it was implemented it has always been problematic and never really used and hence is a legacy implementation detail that needs to be supported but in reality the vast majority of NEON optimisation happens at run time in libraries where it make a difference. In the case of the 'c' crypto extenions we have explicitly not added this as package management functionality as it's handled effectively with run time detection as optimisaiton and as in Fefora has just caused regressions and issues. Fixes RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1691430 Signed-off-by: Peter Robinson <pbrobinson@gmail.com> Closes: #818 Approved by: <try>
💔 Test failed - status-papr |
Merging manually as the automated test failed. |
Thanks @dmach |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We're seeing a few issues on Arm platforms [1], ARMv7 and aarch64, and review there's a few issues and clarifications needed around the way some of the Arm features work. This fixes up the issues seen and clarifies a few details of some of the Arm architecture quirks.
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1562084
Signed-off-by: Peter Robinson pbrobinson@gmail.com