Support for 32-bit arm #196

TheSven73 · 2021-04-15T00:51:53Z

Add Rust support for 32-bit arm.

Fixes: #79
Tested on cortex-a9 (physical hardware) and cortex-a7 (QEMU).

Loose ends:

no pure gcc support yet

v1 -> v2:

trim arm-release defconfig to bring build time to ~10mins
add arm-debug defconfig
remove size_t/uintptr_t guard in 32-bit arm mode only
skip Rust stack probing on debug builds for now (doesn't work)
enable HAS_RUST only on arm cores compatible with the instruction set generated by rustc (armv6)
added extra intrinsics required for debug build
sort alphabetically where possible

v2 -> v3:

eliminate stack probe module crash on debug builds
document arm architecture limitations
defconfig: switch from SLAB to SLUB
config.debug: enable CONFIG_KASAN

v3 -> v4:

rebased to latest rust branch

ojeda

Tested on cortex-a7 and cortex-a9.

Very nice! Was it in QEMU or in actual hardware by chance?

CI very slow (~50 min to build). Trim the defconfig?

Yeah, please copy the other architectures -- you will see they are pretty minimal for this reason (but we do want to test defconfig soon-ish).

no debug config yet (what should it include?)

Ditto.

ojeda · 2021-04-15T00:54:35Z

init/Kconfig

@@ -58,7 +58,7 @@ config LLD_VERSION
 	default 0

 config HAS_RUST
-	depends on ARM64 || (PPC64 && CPU_LITTLE_ENDIAN) || X86_64
+	depends on ARM64 || (PPC64 && CPU_LITTLE_ENDIAN) || X86_64 || ARM


Just a nit: please put it first to keep it the archs alphabetically sorted (same in a few other places).

Suggested change

depends on ARM64 || (PPC64 && CPU_LITTLE_ENDIAN) || X86_64 || ARM

depends on ARM || ARM64 || (PPC64 && CPU_LITTLE_ENDIAN) || X86_64

The most vanilla 32-bit arm triple arm-unknown-linux-gnueabi appears to make Rust generate code compatible with armv6 and up, in non-Thumb mode. I will make sure that HAS_RUST appears only when the kernel is built with a compatible instruction set.

Or, force Rust to generate lowest-common-denominator arm instructions, so it'll run on each and every arm core out there. But that will result in very bad performance on newer cores.

@ojeda we seem to have the following trade-off:
a) use newer instruction set for good performance on newer cores, but won't work on older cores (where HAS_RUST will be automatically disabled)
b) use lowest-common-denominator instruction set which will run on all cores, but results in bad performance on older cores

(this happens because we don't have the ability to mod target.json depending on defconfig. Or do we?)

What is your opinion?

Do not worry too much about that -- we will have the ability to mod the target soon enough. So it is fine picking either one.

I think the lowest-common-denominator makes the most sense since anyhow we are not measuring performance yet, which I assume it is also the easiest of the two alternatives.

Supporting the newer instruction set turned out to be the path of least resistance...

ojeda · 2021-04-15T01:00:18Z

rust/helpers.c

-// See https://github.com/rust-lang/rust-bindgen/issues/1671
-static_assert(__builtin_types_compatible_p(size_t, uintptr_t),
-	"size_t must match uintptr_t, what architecture is this??");


I saw your other PR -- I guess this is fine, but perhaps it doesn't hurt to leave it just in case...

On 32-bit arm, size_t and uintptr_t are incompatible, so the static assert will be triggered. Maybe I should make that clearer in the commit msg.

Under #[cfg] perhaps?

TheSven73 · 2021-04-15T01:18:52Z

Very nice! Was it in QEMU or in actual hardware by chance?

Thanks! cortex-a9 on actual hardware, cortex-a7 on QEMU.

Yeah, please copy the other architectures -- you will see they are pretty minimal for this reason (but we do want to test defconfig soon-ish).

Will do.

no debug config yet (what should it include?)

Ditto.

I will diff debug and release for the other architectures, and use that to generate debug.

ojeda · 2021-04-16T13:58:30Z

LGTM -- @wedsonaf to make him aware of the couple binder changes.

TheSven73 · 2021-04-16T14:03:46Z

LGTM -- @wedsonaf to make him aware of the couple binder changes.

Thank you!! Can you wait until I push v3? I'm going to get rid of the stack probe crash, and add KASAN to the debug config (it's currently missing for some reason).

ojeda · 2021-04-16T14:13:26Z

Sure, no rush! Take as much time as you need.

TheSven73 · 2021-04-16T15:29:56Z

@ojeda this should be good to go from my end. Wait for @wedsonaf 's opinion on the binder changes?

wedsonaf · 2021-04-16T15:35:45Z

@ojeda this should be good to go from my end. Wait for @wedsonaf 's opinion on the binder changes?

Thanks for the ping. I'm happy with the changes (these should eventually move into the kernel crate). Ship it!

ojeda · 2021-04-16T15:53:05Z

It seems you have re-applied/rebased two existing commits in the repo and GitHub's diff gets confused.

TheSven73 · 2021-04-16T17:06:54Z

It seems you have re-applied/rebased two existing commits in the repo and GitHub's diff gets confused.

rust/rust probably moved on while I was working on the PR. I'll rebase and push as v4.

This is simply based on output from: $ rustc +nightly -Z unstable-options \ --target=arm-unknown-linux-gnueabi \ --print target-spec-json Changes made: - removed "unsupported-abis" (does not build) - added "function-sections": false (all kernel functions should go in the .text section) - added "relocation-model": "static" (kernel does not support .data.rel.ro sections) Note that the vanilla `arm-unknown-linux-gnueabi` triple lets rustc generate the armv6 instruction set. Signed-off-by: Sven Van Asbroeck <thesven73@gmail.com>

On 32-bit arm, `size_t` and 'uintptr_t` are incompatible, which will trigger the static assertion. The bug which this guard protects against (rust-lang/rust-bindgen#1671) was fixed upstream as of rust-bindgen v0.53: rust-lang/rust-bindgen#1688 d650823839f7 ("Remove size_t to usize conversion") The current recommended rust-bindgen version for building the Linux kernel is v0.56, so the guard can be safely dropped. Out of an abundance of caution, remove the guard only if building for 32-bit arm. Signed-off-by: Sven Van Asbroeck <thesven73@gmail.com>

Add panic handlers for the following 32-bit arm intrinsics: - __aeabi_ul2f, __aeabi_ul2d, __aeabi_{f,d}cmpeq : floating-point operations - __aeabi_uldivmod, __mulodi4 : 64-bit division Neither floating point operations, nor native 64-bit division are supported inside the kernel, so we can safely implement these with a panic handler. Signed-off-by: Sven Van Asbroeck <thesven73@gmail.com>

Remove the assumption that c_types::c_ulong is u64. This is true only on 64-bit cpu architectures. Signed-off-by: Sven Van Asbroeck <thesven73@gmail.com>

Update the arch-support Rust documentation for arm 32-bit. Signed-off-by: Sven Van Asbroeck <thesven73@gmail.com>

Everything is now in place to enable Rust support on 32-bit arm. Enable only on architectures compatible with the armv6 instruction set. Signed-off-by: Sven Van Asbroeck <thesven73@gmail.com>

qemu emulates the virt machine running a cortex-a7. kernel-arm-release.config corresponds to: - arch/arm/configs/realview_defconfig (includes cortex-a7 virt support) - remove unused kernel infrastructure and drivers - enable rust support, enable rust samples (as modules) - enable android/binder rust support - switch from SLAB to SLUB kernel-arm-debug.config corresponds to: - kernel-arm-release.config - add debug settings under "Kernel Hacking", borrowed from arm64 - use RUST_OPT_LEVEL_2 to prevent stack overflows - enable CONFIG_KASAN Signed-off-by: Sven Van Asbroeck <thesven73@gmail.com>

TheSven73 · 2021-04-16T18:50:00Z

Not sure how rude it is to push that "request review" button! No rush @ojeda, it's the weekend, enjoy !!

ojeda · 2021-04-16T18:58:58Z

It is fine to push it as long as you do not expect a very quick reply ;-P

TheSven73 · 2021-04-16T19:02:50Z

Awesome!! Thank you so much @ojeda and @wedsonaf for reviewing !!

ojeda · 2021-04-16T19:04:03Z

Thanks to you for the work!

TheSven73 · 2021-04-16T19:06:58Z

If you guys like a helping hand with anything else, do let me know. It feels kind of rude to just grab something that's open, without communicating to you folks first.

ojeda · 2021-04-16T19:14:13Z

No worries! If you want to get involved more, the best is that you come to the meetings (join the mailing list to be aware of them, it is a low volume list so far). We will likely have them every week or two weeks.

Otherwise, feel free to pick one of the issues (though ones that are about implementing something new is usually best to ping before in case someone is doing it) or work on something you like even if it is not there yet. Testing and reviewing other PRs is also very welcome.

TheSven73 · 2021-04-18T17:42:13Z

Thanks for the warm welcome! Just out of curiosity, which organisations/people are the driving force behind Rust-for-Linux? Judging by the ambitious scope and activity, I'm assuming that this isn't "just" a 20% project to the key contributors.

ojeda · 2021-04-18T18:05:57Z

Actually, it has been a hobby project for most of us!

ojeda reviewed Apr 15, 2021

View reviewed changes

TheSven73 force-pushed the rust-for-linux-arm-32bit branch 4 times, most recently from d94b886 to 79c154c Compare April 15, 2021 19:46

TheSven73 force-pushed the rust-for-linux-arm-32bit branch from 79c154c to b692fbe Compare April 16, 2021 15:20

Sven Van Asbroeck added 7 commits April 16, 2021 13:12

rust/binder: remove dependency on 64-bit cpu architectures

42c1351

Remove the assumption that c_types::c_ulong is u64. This is true only on 64-bit cpu architectures. Signed-off-by: Sven Van Asbroeck <thesven73@gmail.com>

rust/arm 32-bit: update Documentation

e627aba

Update the arch-support Rust documentation for arm 32-bit. Signed-off-by: Sven Van Asbroeck <thesven73@gmail.com>

rust: enable for 32-bit arm

c4f1bde

Everything is now in place to enable Rust support on 32-bit arm. Enable only on architectures compatible with the armv6 instruction set. Signed-off-by: Sven Van Asbroeck <thesven73@gmail.com>

TheSven73 force-pushed the rust-for-linux-arm-32bit branch from b692fbe to 7e0815b Compare April 16, 2021 17:40

TheSven73 requested a review from ojeda April 16, 2021 18:48

ojeda merged commit 30d13c6 into Rust-for-Linux:rust Apr 16, 2021

TheSven73 deleted the rust-for-linux-arm-32bit branch April 16, 2021 19:03

geofft mentioned this pull request Jun 3, 2021

On the size_t / uintptr_t guard #329

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for 32-bit arm #196

Support for 32-bit arm #196

TheSven73 commented Apr 15, 2021 •

edited

Loading

ojeda left a comment

ojeda Apr 15, 2021

TheSven73 Apr 15, 2021 •

edited

Loading

TheSven73 Apr 15, 2021 •

edited

Loading

ojeda Apr 15, 2021 •

edited

Loading

TheSven73 Apr 15, 2021

ojeda Apr 15, 2021

TheSven73 Apr 15, 2021 •

edited

Loading

ojeda Apr 15, 2021

TheSven73 commented Apr 15, 2021

ojeda commented Apr 16, 2021

TheSven73 commented Apr 16, 2021

ojeda commented Apr 16, 2021

TheSven73 commented Apr 16, 2021

wedsonaf commented Apr 16, 2021

ojeda commented Apr 16, 2021

TheSven73 commented Apr 16, 2021

TheSven73 commented Apr 16, 2021

ojeda commented Apr 16, 2021

TheSven73 commented Apr 16, 2021

ojeda commented Apr 16, 2021

TheSven73 commented Apr 16, 2021

ojeda commented Apr 16, 2021 •

edited

Loading

TheSven73 commented Apr 18, 2021

ojeda commented Apr 18, 2021

	depends on ARM64 \|\| (PPC64 && CPU_LITTLE_ENDIAN) \|\| X86_64 \|\| ARM
	depends on ARM \|\| ARM64 \|\| (PPC64 && CPU_LITTLE_ENDIAN) \|\| X86_64

Support for 32-bit arm #196

Support for 32-bit arm #196

Conversation

TheSven73 commented Apr 15, 2021 • edited Loading

ojeda left a comment

Choose a reason for hiding this comment

ojeda Apr 15, 2021

Choose a reason for hiding this comment

TheSven73 Apr 15, 2021 • edited Loading

Choose a reason for hiding this comment

TheSven73 Apr 15, 2021 • edited Loading

Choose a reason for hiding this comment

ojeda Apr 15, 2021 • edited Loading

Choose a reason for hiding this comment

TheSven73 Apr 15, 2021

Choose a reason for hiding this comment

ojeda Apr 15, 2021

Choose a reason for hiding this comment

TheSven73 Apr 15, 2021 • edited Loading

Choose a reason for hiding this comment

ojeda Apr 15, 2021

Choose a reason for hiding this comment

TheSven73 commented Apr 15, 2021

ojeda commented Apr 16, 2021

TheSven73 commented Apr 16, 2021

ojeda commented Apr 16, 2021

TheSven73 commented Apr 16, 2021

wedsonaf commented Apr 16, 2021

ojeda commented Apr 16, 2021

TheSven73 commented Apr 16, 2021

TheSven73 commented Apr 16, 2021

ojeda commented Apr 16, 2021

TheSven73 commented Apr 16, 2021

ojeda commented Apr 16, 2021

TheSven73 commented Apr 16, 2021

ojeda commented Apr 16, 2021 • edited Loading

TheSven73 commented Apr 18, 2021

ojeda commented Apr 18, 2021

TheSven73 commented Apr 15, 2021 •

edited

Loading

TheSven73 Apr 15, 2021 •

edited

Loading

TheSven73 Apr 15, 2021 •

edited

Loading

ojeda Apr 15, 2021 •

edited

Loading

TheSven73 Apr 15, 2021 •

edited

Loading

ojeda commented Apr 16, 2021 •

edited

Loading