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.

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

Fixed a bug that the little-endian Microblaze does not work when MBEDTLS_HAVE_ASM is defined #4586

Closed

Conversation

Kazuyuki-Kimura
Copy link

@Kazuyuki-Kimura Kazuyuki-Kimura commented May 31, 2021

Description

Fixed a bug that the little-endian Microblaze does not work when MBEDTLS_HAVE_ASM is defined.
Fixes issue #2020

Status

READY

Requires Backporting

Yes
Which branch?
2.16

Migrations

NO

@Kazuyuki-Kimura Kazuyuki-Kimura force-pushed the development branch 3 times, most recently from 9067a32 to d027a88 Compare June 1, 2021 02:28
@Kazuyuki-Kimura Kazuyuki-Kimura marked this pull request as draft June 1, 2021 08:35
@Kazuyuki-Kimura Kazuyuki-Kimura marked this pull request as ready for review June 1, 2021 08:35
@mpg
Copy link
Contributor

mpg commented Jun 16, 2021

Hi @Kazuyuki-Kimura and thanks for your contribution! Were you able to reproduce the bug and verify your fix on the affected platform?

@mpg mpg added bug Community needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review labels Jun 16, 2021
@Kazuyuki-Kimura
Copy link
Author

Without knowing about this bug, I installed mbedTLS in a mail client I was making, and it stopped just like #2020, so I think I was able to reproduce it.
When I wrote a new assembler code to improve the speed and pass the test, I compared the results and found that the only difference was the order in which the 64-bit data was imported from memory.
microblaze allows you to choose the endianness when you embed it in your FPGA.
When we first released it, I think we created the code to work with big-endian cores.
We have passed all the necessary tests and confirmed that we can send emails to the SMTPS server without any problems.

@mpg
Copy link
Contributor

mpg commented Jun 18, 2021

Thanks ! Since we don't have the relevant hardware for testing ourselves, it's important to know that your patch was tested.

Copy link
Contributor

@mpg mpg left a comment

Choose a reason for hiding this comment

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

The patch looks good to me, just one nit-pick: could you rebase on top of current development in order to avoid the merge commits? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug needs-backports Backports are missing or are pending review and approval. needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants