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

Fix compatibility with big endian hosts #70

Closed
wants to merge 1 commit into from

Conversation

zeldin
Copy link

@zeldin zeldin commented Dec 11, 2020

The Pi bootcode expects the boot_message.length field to be sent in
little endian, and sends the message.command in little endian.
Make sure they are converted from whatever the host is using.

@pelwell
Copy link
Contributor

pelwell commented Dec 11, 2020

Out of curiosity, what is the use case that triggered this PR? You don't see much big-endian hardware these days.

@zeldin
Copy link
Author

zeldin commented Dec 11, 2020

@pelwell My main desktop computer is a Talos II (POWER9). But I also happen to run my Raspberry Pi 3 in big endian. :-)
https://github.com/zeldin/linux-1/releases
Now that I have a CM4 IO board with a PCIe slot, I hope to be able to debug the PCIe driver so that I can run Pi 4 in big endian mode as well...

@pelwell
Copy link
Contributor

pelwell commented Dec 11, 2020

Having grown up on a 68000, little-endian was an adjustment, but I prefer it now.

usbboot isn't a paragon of programming style, but I think a better fix would be to declare the length field of bootmsg as an array of 4 uint8_t/unsigned chars and add another global variable - bootmsg_length, say - that holds the value in the natural endianness. You then only need to serialise just after receiving or just before sending, which doesn't need the union since you are working with an array.

The Pi bootcode expects the boot_message.length field to be sent in
little endian, and sends the message.command in little endian.
Make sure they are converted from/to whatever the host is using.
@zeldin
Copy link
Author

zeldin commented Dec 11, 2020

Sure. That also covers the case if int has a different size than 32 bits. I initially assumed you wanted to stay with C90 because I didn't see an include of <stdint.h> and the length/command fields were int rather than int32_t, but I do see now that C99 types are used in other places of main.c. I'll add the include line while I'm at it. 😄

@timg236
Copy link
Collaborator

timg236 commented Nov 5, 2024

Closing as stale PR

@timg236 timg236 closed this Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants