-
Notifications
You must be signed in to change notification settings - Fork 734
The beginnings of Rust support for LK #479
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
Conversation
|
Starting a review now, uno momento. |
travisg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First pass of looking at it. I haven't tried to build it locally, will give that a whirl in the next few days.
Looks like a good start.
Also a thing we should do is perhaps extend the .github CI rules to try to build this, perhaps with a new special CI that isn't really blocking while things get sorted out.
| MODULE_BUILDDIR := $(call TOBUILDDIR,$(MODULE_SRCDIR)) | ||
|
|
||
| # TODO These should be set by the arch. | ||
| RUST_TARGET := aarch64-unknown-none |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: figure out how we can define our own triple and build the core/alloc crates ourselves, as we talked about a bit the other day. The generation of floating point code with the default triple is already technically a major problem as it stands, since there's nothing that keeps LLVM from generating floating point code in the pl011 irq handler with the standard triple.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have this mess from Zephyr: https://github.com/zephyrproject-rtos/zephyr-lang-rust/blob/161825c50a7870017e52afc29169816c7f4500b4/CMakeLists.txt#L14-L59, and there is a PR to address calculating the risc-v version. I think we have it easier since each platform can actually define proper tuple. I can look further into building core/alloc and defining our own tuples. At this point, this will make us depend on the nightly compiler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would also like to address doing this in a more complete way with an additional PR(s).
| LK_INIT_HOOK!( | ||
| KERNEL_LOG_INIT, | ||
| kernel_log_init_func, | ||
| lk_init_level::LK_INIT_LEVEL_ARCH |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm guessing this could actually be somewhat earlier, probably INIT_LEVEL_THREADING?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It needs alloc to work, and whatever is needed for stdout/fputs to work.
rust/lk/src/lib.rs
Outdated
| /// | ||
| /// Due to linker optimization, without a direct call into this code, the linker will not bring in | ||
| /// anything. | ||
| pub fn init() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though it's not doing much, I think i mentioned it in the platform code, we should probably call this as soon as possible, so that any rust code that comes online later can just assume any background rust setup has already happened. OTOH, this fputs may be assuming there's already some sort of log path available? May need to init silently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Realistically, this probably shouldn't even be called 'init', since it isn't actually doing anything, just making sure the code is linked in. The init can happen through the normal lk init mechanism.
f5cbf88 to
7e36a62
Compare
| // version than the version used to compile the code. We're making a basic assumption that the | ||
| // code is either being built with LLVM, or that GCC and LLVM are compatible enough for this to | ||
| // generate correct bindings. | ||
| let version = bindgen::clang_version(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to point bindgen to the path of clang/llvm we want to use? We don't have to do it in this pull request
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bindgen will use LIBCLANG_PATH, in fact, I'll need to set that for this to work on MacOS, as it seems to default to the fairly old clang that MacOS provides. It dynamically loads libclang.
platform/qemu-virt-arm/rules.mk
Outdated
| CONSOLE_HAS_INPUT_BUFFER=1 \ | ||
| TIMER_ARM_GENERIC_SELECTED=CNTV | ||
| TIMER_ARM_GENERIC_SELECTED=CNTV \ | ||
| USE_RUST=$(USE_RUST) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add an \ character at end of last item? This makes appending items to this list easier
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem to be the convention throughout the other makefiles, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it's al over the place, newer makefiles I've been generally just puttng the same variable over and over again with a +=. Someone told me a while back that make actually has a really inefficient mechanism to deal with , by basically reading a new line, reallocating it to append more data on it. Of course it doesn't really matter.
lib/rust_support/rules.mk
Outdated
| echo "# AUTOGENERATED -- edit $(MODULE_SRCDIR)/rules.mk instead"; \ | ||
| echo "[build]"; \ | ||
| echo 'target = "$(RUST_TARGET_PATH)"'; \ | ||
| echo 'rustflags = [ "-Ctarget-feature=-neon"]'; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I lack enough background knowledge, can we leave some comment in code to explain why we need to not mess with floating point registers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've removed this line, as I've figured out how to put this in the real target support. It does make sense to describe the platform there, but I'm not sure. There are a few specific things, such as no FP hardware, and having to tell the compiler to not use the x18 register, as LK now uses that as a global per-CPU pointer.
This brings in basic support for building Rust code as part of LK. The build system compiles a single Rust crate (using Cargo) into a static library. The crate is a template, to deal with the assumption Cargo makes about the top-level crate directing the configuration of the entire build. As part of the build, these files are copied into the build directory, as well as a `.cargo/config.toml` file that sets up the compilation for this build with the rust compiler. By doing it this way, it is simply a matter of pointing a tool, such as vscode, to this directory, and the rust-analyzer will work as expected. Signed-off-by: David Brown <david.brown@linaro.org>
The lk-sys crate provides low-level bindings to LK functions and types. This runs bindgen in build.rs, and includes the generated output in the main library. In addition, there are a small number of instances that need to be in this file, as these must be in the same crate as the definitions. Signed-off-by: David Brown <david.brown@linaro.org>
The lk crate provides the basic Rust-friendly interfaces to LK. At this
point, the following are available:
- init: Interface to LK's init mechanism
- log: Provides an implementation of the 'log' crate that calls fputs
in lk.
- lkonce: A run-once primitive, similar to std::sync::Once, but with
provision for using zero-initialized memory, with the initialization
being called through the lk init mechanism.
- macros: A few useful macros around `container_of`.
Signed-off-by: David Brown <david.brown@linaro.org>
Instead of just always enabling Rust, only enable if it is specifically requested via the `USE_RUST` variable. Signed-off-by: David Brown <david.brown@linaro.org>
This is a Rust reimplementation of the PL011 UART driver. This will be used in any build that requests the pl011 driver and has rust support. Signed-off-by: David Brown <david.brown@linaro.org>
llvm-objdump prints a non-suppressible warning for any source files mentioned in objects that aren't present. As we are using the pre-compiled Rust core/alloc libraries, which have a hard-coded path `rustc/<hash>/...` for these source files, and it is up to the user whether the source package is present (and to make a symlink so they can be found), just capture and filter out these messages. This should only be filtering out the specific message. Signed-off-by: David Brown <david.brown@linaro.org>
As LK contains a directory called `target/` we can't ignore this at the top level. Ignore this directory and Cargo.lock only within the rust directory. Currently all crates live here, so this should keep target from showing up to git. The real target directory will be under the build-* directory, which is already ignored. Signed-off-by: David Brown <david.brown@linaro.org>
When referencing other crates from the `rust_support/Cargo.toml` file, use the `BUILDROOT` instead of a large relative path. This makes the process not depend on where the build directory is. Signed-off-by: David Brown <david.brown@linaro.org>
Remove a redundant MODULE_OBJECT assignment, and add this module to the MODULES list. Signed-off-by: David Brown <david.brown@linaro.org>
Signed-off-by: David Brown <david.brown@linaro.org>
Update all of the new Rust source files with an MIT header (from other parts of LK). Add the license declaration line to the various Cargo.toml files. Signed-off-by: David Brown <david.brown@linaro.org>
Instead of depending on a pre-defined target tuple, with a pre-build core and allow, capture our configuration with a custom LLVM target JSON file. To do this, we need to build core and alloc, which currently requires a nightly compiler. The advantage here is that core and alloc will be build with the exact configuration we define here, including any specific command line arguments. Signed-off-by: David Brown <david.brown@linaro.org>
Ensure that the generated aarch64 rust code does not make use of floating point registers, pass this option in vai RUSTCFLAGS variable. This will need more general support to add other architectures. Signed-off-by: David Brown <david.brown@linaro.org>
As the builds now require the nightly compiler, install that before trying to build with CI. Signed-off-by: David Brown <david.brown@linaro.org>
Base this json file off of aarch64-unknown-none-softfloat, to properly disable use of HW floating point registers in the kernel. This avoids needing to explicitly pass and option to avoid using Neon. Note that at the time of this commit, building core with this configuration generates a warning, due to some explicitly neon code in core. Presumably, this will be fixed in core at least before the compiler starts considering this to be an error. Signed-off-by: David Brown <davidb@davidb.org>
Remove the "Trusty" name as this code is more generally to LK. Signed-off-by: David Brown <davidb@davidb.org>
Cleanly separate enablement of Rust support from a `USE_RUST` that can be used when invoking make, and `HAVE_RUST` that is defined internally when rust support is enabled. This allows targets to decide for themselves if Rust support is needed, or if it is optional. The definitive decision about rust support will be determined if a platform makefile includes `lib/rust_support/rules.mk` to include Rust support in the build. It can optional do this based on a use-defined `USE_RUST` flag. Signed-off-by: David Brown <davidb@davidb.org>
The variable RUST_CRATES contains a list of paths (from the build root) to crates to add. For each crate: generate a call to `cratename::must_link()` to force the crate to be linked in, allowing lk init blocks to be included. This makes the pl011 crate a module, so the crate will be included when the module is included. Signed-off-by: David Brown <davidb@davidb.org>
This creates an very basic rust hello world app. The app declaration is done through unsafe C, but this is sufficient to demonstrate building and linking a rust app into LK. Signed-off-by: David Brown <davidb@davidb.org>
Create a framework for defining LK applications in Rust, simplifying app creation and management. The `LkApp` trait can be applied to a type, with the `lkapp!` macro then generating the boilerplate needed to integrate the app with LK. Signed-off-by: David Brown <davidb@davidb.org>
d3zd3z
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@travisg I think this is pretty close to ready for an initial version. Let me know if there is anything you think I should address in this PR rather than additional PRs.
|
I guess the one thing that maybe I should look into first would be making this build on some other targets. But that might also make more sense as additional patches. |
|
I have a question, does this mean that new functionality should be implemented using Rust first? I'm asking because I would like to add MMC/SD card API support and first driver is pl180 for Arm FVP emulator, and there's an implementation written in C (WIP). So I'm wondering if it's worth trying to rewrite it using Rust. |
This is not an attempt to replace C code in LK with rust, or enforce new code to be written in rust. This effort is only meant to enable Rust code for those who need. |
|
Looking at it now, so far everything is looking great, though I think I can tweak some of the build system stuff to make it a little more conditional and a few less cross-module deps in the C world. Would you mind if i just pushed a change or two on top of this stack? |
Allows the overall logic of the build system to only include the module once, and allow other parts of the build system such as adding include directories. Otherwise none of the rest of the module logic is used, such as the generation of the local config.mk or adding any .o file to the final system linkage.
An anchor function reference is needed to make sure the linker includes things from the rust .a files. Previously was defined in qemu-virt-arm64 but since rust is a general feature, move it into top/main.c
-Separate the two pl011 drivers so they do not need to be aware of each
other:
- make sure the platform selects the right one
- have the rust one make a copy of the header for C code to interact
with
- Make the top level rust modules (app/rust_hello, rust/dev-pl011) use
the LK module.mk which can now deal with empty modules. This cleans up
the deps a little bit and uses existing machinery to add include
paths.
- Move the logic to select rust modules into the qemu-virt-arm64 project
and have it conditionally include a new virtual project file that
encapsulates rust deps. Will need to refacto this a bit later as more
platform/projects make use of rust, but at least moves it up to the
project level instead of down in a single platform.
|
I added a handful of build system and structural changes over at https://github.com/littlekernel/lk/tree/linaro-rust-additions that add to the top of this. @d3zd3z can you take a quick look at these and see if they're acceptable? There might be a better way of stacking changes on changes in github, but I have no idea. I'm so used to gerrit where this sort of thing is no big thang. |
d3zd3z
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@travisg the changes look fine. I'm brought them in, and the checks all pass.
|
Looks good, going to merge in. Thanks for the work! This is pretty exciting, will have to figure out how to expand to more uses while keeping it (at the moment) non essential due to nightly rust requirements, etc. |
This adds the basics of Rust support for lk. Notably, it includes: - A pl011 driver written in Rust. - Interfaces to init, log, cbuf in lk. - A type 'LkOnce' that provides Once-like functionality but in a way that works with LK's init mechanism. - Rust code is built as a single build with cargo to generate a .a file linked into the build. The Rust support is not enabled by default, and can be enabled by passing USE_RUST=1 to the make invocation. Currently only qemu-virst-arm64-test is supported, and this has only been tested when using clang to compile the C code. PR #479
This adds the basics of Rust support for lk. Notably, it includes:
cargoto generate a .a file linked into the build.The Rust support is not enabled by default, and can be enabled by passing
USE_RUST=1to the make invocation. Currently only qemu-virst-arm64-test is supported, and this has only been tested when using clang to compile the C code.