64 bit: Initial support for Linux x86-64 native.#19890
64 bit: Initial support for Linux x86-64 native.#19890fzi-haxel wants to merge 3 commits intoRIOT-OS:masterfrom
Conversation
fzi-haxel
requested review from
MichelRottleuthner,
MrKevinWeiss,
aabadie,
bergzand,
fjmolinas,
haukepetersen,
jia200x,
kaspar030,
leandrolanzieri,
miri64 and
smlng
as code owners
github-actions
bot
added
Platform: native
benpicco
left a comment
benpicco
left a comment
There was a problem hiding this comment.
If you move the preparatory patches (64 bit safety compile fixes) to a separate PR we can merge those quickly.
For changes to third party packages, please try to also make a PR upstream so we can drop the downstream patch next time we update the pkt version.
Then you'd only have changes to native left in this PR, which makes review easier (and reduces the chance of merge conflicts).
| kv->len = kv_hdr.len; | ||
|
|
||
| - if (kv->len == ~0UL || kv->len > db_max_size(db) || kv->len < KV_HDR_DATA_SIZE) { | ||
| + if (kv->len == UINT32_MAX || kv->len > db_max_size(db) || kv->len < KV_HDR_DATA_SIZE) { |
There was a problem hiding this comment.
upstream is pretty responsive when it comes to patches, you can just open a PR there and we can update the commit hash.
| { | ||
| (void) ccnl; | ||
| - DEBUGMSG(DEBUG, "Received something of size %u for the application\n", c->pkt->contlen); | ||
| + DEBUGMSG(DEBUG, "Received something of size %u for the application\n", (unsigned)c->pkt->contlen); |
There was a problem hiding this comment.
You can also move that upstream
|
Is this based on #13009 or a complete rework? |
|
probably this won't make it into the release. |
| stm32mp157c-dk2 \ | ||
| yunjia-nrf51822 \ | ||
| waspmote-pro \ | ||
| z1 \ |
There was a problem hiding this comment.
How are these changes related to this PR?
There was a problem hiding this comment.
While replacing most of FEATURES_REQUIRED += arch_32bit with FEATURES_REQUIRED_ANY += arch_32bit|arch_64bit to make sure the same tests are run for 64bit, I found this one unnecessary.
So I deleted it instead of adding arch_64bit. But good point, this could also be its own PR.
github-actions
bot
removed
Area: LoRa
|
I changed the PR to WiP and updated the commits so people can still test 64bit native. Updates include:
|
github-actions
bot
removed
Area: core
Fixed compilation errors. Mostly DEBUG/printf formatting and void pointer casting. Other changes are: * net/gnrc_sixlowpan_frag_*: Generalized packet size calculation * cpu/native_backtrace: Reduced required backtrace size to 3 for 64-bit * periph/flashpage: Simplified test * unittests/tests-pktbuf: Generalized alignment * sys/architecture: Extended test for 64-bit
Initial version to test 64 bit compatibility.
Instead of a separate board, 64 bit for Linux/x86_64 is enabled by setting the environment variable `NATIVE_64BIT=y` and compiling as usual.
While I personally prefer a separate `native64` board, this ensures that all tests run with the same configuration as the 32 bit version.
A separate board would require a major refactoring of many tests, which often have special behavior for the native board,
and I didn't want to increase the size of an already large pull request.
Not currently implemented:
* Architectures other than x86_64 or operating systems other than Linux
* No FreeBSD support
* No Aarch support
* Rust support for x86_64
|
Closing this PR. The PRs above have been merged. |
4 participants
Contribution description
Initial support for 64 bit in the kernel, pkgs, examples, tests and the native board.
Why add 64 bit support?
As mentioned in the FAQ on www.riot-os.org, there are some benefits
The FAQ also mentions some cons
sizeof(void*) == sizeof(size_t) != sizeof(int), which leads to quite a few compiler warnings in printf formatting and casting between integer values and pointers. This is especially true for the tests. Other than that, and 64 bit support for the native board, the rest of the changes are fairly minor. I also tried to change as little as possible to avoid breaking compatibility with the other boards. See the commit messages for more details.Native for Linux x86-64
For the initial version, instead of creating a separate
native64board, I decided to add the variableNATIVE_64BITto the native board Makefiles. Setting the environment toNATIVE_64BIT=ywill enable 64-bit mode for native, which is disabled by default.While I don't necessarily think this is the best long-term solution, it does ensure that all tests run with the same configuration as the 32 bit version. A separate board would require a major refactoring of many tests, which often have different configurations in the Makefile, C code, and Python tests if the board is native, and I didn't want to increase the size of an already large pull request.
Almost all of the same features as the 32 bit version are supported by the 64 bit version, but the following are still missing
Some closing thoughts
This PR is a suggestion for initial 64 bit support, feel free to suggest any changes.
I thought about making several smaller PRs, but that would have made it difficult to test the 64 bit part.
So I decided to go for the big bang solution. If you have a better suggestion, I'm open to restructuring the PR.
DISCLAMER: While I did a more thorough review of the kernel, I did not do a thorough review of the entire code base. I mainly fixed compilation or testing errors that came up during testing. I am sure I have missed at least some DEBUG/printf formatting issues. There is also some optimization potential for 64 bit, but I focused on minimal changes.
Testing procedure
Just set the
NATIVE_64BIT=yenvironment variable and compile something for native.For example
NATIVE_64BIT=y ./dist/tools/compile_and_test_for_board/compile_and_test_for_board.py . nativeNOTE:
NATIVE_64BIT=ywill skip the unsupported Rust examples and tests. Other than that, nothing else should be skipped.libsdl2-devfor 64 bit.Issues/PRs references
See also #6603