[ojeda]

Currently we have several lines of work:

• Integrating with the kernel tree and build system (Nick's & mine, both uploaded as branches, based on rustc; and another one I have been working on, based on cargo).
• Bindings and the first bits of functionality (Alex's & Geoffrey's, based on cargo).

This patch effectively merges the work we have been doing and integrates it in the latest mainline kernel tree.

This does not mean anything needs to stay as-is, but this gives us a working, common base to work and experiment upon. Hopefully, it will also attract external people to join!

As a summary, I added:

• cargo integration with the kernel Makefiles:

  • Virtual cargo workspace to have a single lock file and to share deps between cargo jobs.
  • Picks the same optimization level as configured for C.
  • Verbose output on V=1.
  • A cargoclean target to clean all the Rust-related artifacts.

• Initial support for built-in modules (i.e. Y as well as M):

  • It is a hack, we need to implement a few things (see the TODOs), but it is enough to start playing with things that depend on MODULE.
  • Passes --cfg module to built-in modules to be able to compile conditionally inside Rust.
  • Increased KSYM_NAME_LEN length to avoid warnings due to Rust long mangled symbols.

• Rust infrastructure in a new top level folder rust/:

  • A kernel package which contains the sources from Alex & Geoffrey's work plus some changes:
    • Adapted build.rs.
    • Removed the THIS_MODULE emulation until it is implemented.
    • Removed Makefile logic and the code that cfg-depended on kernel version (no need in mainline).
    • Moved the helpers to be triggered via normal compilation, renamed them under rust_* and exported via EXPORT_SYMBOL instead.
    • Added a prelude.
  • A shlex package which serves as an example of an "inline" dependency (i.e. package checked out copy to avoid the network)

• The example driver was setup at drivers/char/rust_example/.

• Misc

  • The beginning of Documentation/rust/ with a quick start guide.
  • MAINTAINERS entry.
  • SPDXs for all files.

Other notes that aren't in TODOs:

• We could minimize the network requirements (use bindgen binary, use more inline dependencies...), but it is not clear it would be a good idea for the moment due to core/alloc/compiler-builtins.

• The intention of rust/ is to have a place to put extra dependencies and split the kernel package into several in the future if it grows. It could resemble the usual kernel tree structure.

• With several drivers being built-in, cargo recompiles kernel and triggers duplicate symbol errors when merging thin archives. We need to make it only compile kernel once.

• When the above works, then make's -j calling concurrent cargos (e.g. several drivers at the same time) should be OK, I think. According to Fix running Cargo concurrently rust-lang/cargo#2486 it shouldn't miscompile anything, but if they start locking on each other we will have make jobs waiting that could have been doing something else.

Signed-off-by: Miguel Ojeda miguel.ojeda.sandonis@gmail.com

• I added TODOs in some places of the code I touched, so take a look.
• shlex is just an example (most likely we will let cargo do its job instead).
• Concerning credit (especially for Geoffrey & Alex): I wrote "Rust for Linux Contributors" everywhere for the moment, but do not worry: we will mention everyone in the eventual PR, note individual contributions (including per patch/file if needed), etc.

I didn't review the code that was directly copied too much. And my Makefile skills are subpar -- I barely understand how this KBuild integration works! But overall this looks amazing, thanks for driving this forward.

I'm not worried about credit at all. I'm so excited to see this moving forward!

I didn't review the code that was directly copied too much.

Yeah, I could have put an initial commit with just those files to make it show easier on GitHub's interface. Let me paste the manual diff here instead.

And my Makefile skills are subpar -- I barely understand how this KBuild integration works!

I don't think anybody understands it... ;) Later on we can ask Masahiro to give us a hand (e.g. to remove the hack I put there to make the distinction between built-in and module).

But overall this looks amazing, thanks for driving this forward.

You're welcome! And thanks for taking a look so quickly :-)

@alex @geofft The manual diff between the original files and the new ones (minus the repetitive SPDX ones):

diff -ur ./bindings_helper.h ../../linux/rust/kernel/src/bindings_helper.h
@@ -6,6 +8,5 @@
 #include <linux/uaccess.h>
 #include <linux/version.h>
 
-// Bindgen gets confused at certain things
-//
+// bindgen gets confused at certain things
 const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
diff -ur ./bindings.rs ../../linux/rust/kernel/src/bindings.rs
@@ -5,10 +7,10 @@
     non_snake_case,
     improper_ctypes
 )]
-mod bindings {
+mod bindings_raw {
     use crate::c_types;
-    include!(concat!(env!("OUT_DIR"), "/bindings.rs"));
+    include!("bindings_gen.rs");
 }
-pub use bindings::*;
+pub use bindings_raw::*;
 
 pub const GFP_KERNEL: gfp_t = BINDINGS_GFP_KERNEL;
diff -ur ./chrdev.rs ../../linux/rust/kernel/src/chrdev.rs
@@ -56,7 +58,8 @@
         for (i, file_op) in self.file_ops.iter().enumerate() {
             unsafe {
                 bindings::cdev_init(&mut cdevs[i], *file_op);
-                cdevs[i].owner = &mut bindings::__this_module;
+                // TODO: proper `THIS_MODULE` handling
+                cdevs[i].owner = core::ptr::null_mut();
                 let rc = bindings::cdev_add(&mut cdevs[i], dev + i as bindings::dev_t, 1);
                 if rc != 0 {
                     // Clean up the ones that were allocated.
diff -ur ./file_operations.rs ../../linux/rust/kernel/src/file_operations.rs
@@ -160,18 +162,10 @@
             None
         },
 
-        #[cfg(not(kernel_4_9_0_or_greater))]
-        aio_fsync: None,
         check_flags: None,
-        #[cfg(all(kernel_4_5_0_or_greater, not(kernel_4_20_0_or_greater)))]
-        clone_file_range: None,
         compat_ioctl: None,
-        #[cfg(kernel_4_5_0_or_greater)]
         copy_file_range: None,
-        #[cfg(all(kernel_4_5_0_or_greater, not(kernel_4_20_0_or_greater)))]
-        dedupe_file_range: None,
         fallocate: None,
-        #[cfg(kernel_4_19_0_or_greater)]
         fadvise: None,
         fasync: None,
         flock: None,
@@ -179,22 +173,16 @@
         fsync: None,
         get_unmapped_area: None,
         iterate: None,
-        #[cfg(kernel_4_7_0_or_greater)]
         iterate_shared: None,
-        #[cfg(kernel_5_1_0_or_greater)]
         iopoll: None,
         lock: None,
         mmap: None,
-        #[cfg(kernel_4_15_0_or_greater)]
         mmap_supported_flags: 0,
         owner: ptr::null_mut(),
         poll: None,
         read_iter: None,
-        #[cfg(kernel_4_20_0_or_greater)]
         remap_file_range: None,
         sendpage: None,
-        #[cfg(kernel_aufs_setfl)]
-        setfl: None,
         setlease: None,
         show_fdinfo: None,
         splice_read: None,
diff -ur ./filesystem.rs ../../linux/rust/kernel/src/filesystem.rs
@@ -61,7 +63,8 @@
     let mut fs_registration = Registration {
         ptr: Box::new(bindings::file_system_type {
             name: T::NAME.as_ptr() as *const i8,
-            owner: unsafe { &mut bindings::__this_module },
+            // TODO: proper `THIS_MODULE` handling
+            owner: core::ptr::null_mut(),
             fs_flags: T::FLAGS.bits(),
             mount: Some(mount_callback::<T>),
             kill_sb: Some(bindings::kill_litter_super),
diff -ur ./lib.rs ../../linux/rust/kernel/src/lib.rs
@@ -1,3 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! The `kernel` crate
+
 #![no_std]
 #![feature(allocator_api, alloc_error_handler, const_raw_ptr_deref)]
 
@@ -12,8 +16,8 @@
 mod error;
 pub mod file_operations;
 pub mod filesystem;
+pub mod prelude;
 pub mod printk;
-#[cfg(kernel_4_13_0_or_greater)]
 pub mod random;
 pub mod sysctl;
 mod types;
@@ -27,17 +31,18 @@
 ///
 /// Example:
 /// ```rust,no_run
-/// use linux_kernel_module;
+/// use kernel::prelude::*;
+///
 /// struct MyKernelModule;
-/// impl linux_kernel_module::KernelModule for MyKernelModule {
-///     fn init() -> linux_kernel_module::KernelResult<Self> {
+/// impl KernelModule for MyKernelModule {
+///     fn init() -> KernelResult<Self> {
 ///         Ok(MyKernelModule)
 ///     }
 /// }
 ///
-/// linux_kernel_module::kernel_module!(
+/// kernel_module!(
 ///     MyKernelModule,
-///     author: b"Fish in a Barrel Contributors",
+///     author: b"Rust for Linux Contributors",
 ///     description: b"My very own kernel module!",
 ///     license: b"GPL"
 /// );
@@ -45,6 +50,14 @@
 macro_rules! kernel_module {
     ($module:ty, $($name:ident : $value:expr),*) => {
         static mut __MOD: Option<$module> = None;
+
+        // TODO: find a proper way to emulate the C macro, including
+        // dealing with `HAVE_ARCH_PREL32_RELOCATIONS`
+        #[cfg(not(module))]
+        #[link_section = ".initcall6.init"]
+        #[used]
+        pub static __initcall: extern "C" fn() -> $crate::c_types::c_int = init_module;
+
         #[no_mangle]
         pub extern "C" fn init_module() -> $crate::c_types::c_int {
             match <$module as $crate::KernelModule>::init() {
@@ -85,6 +98,7 @@
     // Most of the alternatives (e.g. .as_bytes() as a const fn) give
     // you a pointer, not an array, which isn't right.
 
+    // TODO: `modules.builtin.modinfo` etc. is missing the prefix (module name)
     (@attribute author, $value:expr) => {
         #[link_section = ".modinfo"]
         #[used]
@@ -133,13 +147,13 @@
 }
 
 extern "C" {
-    fn bug_helper() -> !;
+    fn rust_helper_BUG() -> !;
 }
 
 #[panic_handler]
 fn panic(_info: &PanicInfo) -> ! {
     unsafe {
-        bug_helper();
+        rust_helper_BUG();
     }
 }
diff -ur ./random.rs ../../linux/rust/kernel/src/random.rs
@@ -21,9 +23,7 @@
 }
 
 /// Fills `dest` with random bytes generated from the kernel's CSPRNG. If the
-/// CSPRNG is not yet seeded, returns an `Err(EAGAIN)` immediately. Only
-/// available on 4.19 and later kernels.
-#[cfg(kernel_4_19_0_or_greater)]
+/// CSPRNG is not yet seeded, returns an `Err(EAGAIN)` immediately.
 pub fn getrandom_nonblock(dest: &mut [u8]) -> error::KernelResult<()> {
     if !unsafe { bindings::rng_is_initialized() } {
         return Err(error::Error::EAGAIN);
diff -ur ./user_ptr.rs ../../linux/rust/kernel/src/user_ptr.rs
@@ -7,7 +9,7 @@
 use crate::error;
 
 extern "C" {
-    fn access_ok_helper(addr: *const c_types::c_void, len: c_types::c_ulong) -> c_types::c_int;
+    fn rust_helper_access_ok(addr: *const c_types::c_void, len: c_types::c_ulong) -> c_types::c_int;
 }
 
 /// A reference to an area in userspace memory, which can be either
@@ -55,7 +57,7 @@
         ptr: *mut c_types::c_void,
         length: usize,
     ) -> error::KernelResult<UserSlicePtr> {
-        if access_ok_helper(ptr, length as c_types::c_ulong) == 0 {
+        if rust_helper_access_ok(ptr, length as c_types::c_ulong) == 0 {
             return Err(error::Error::EFAULT);
         }
         Ok(UserSlicePtr(ptr, length))

build.rs diff:

@@ -1,6 +1,7 @@
-use std::io::{BufRead, BufReader};
+// SPDX-License-Identifier: GPL-2.0
+
 use std::path::PathBuf;
-use std::{env, fs};
+use std::env;
 
 const INCLUDED_TYPES: &[&str] = &["file_system_type", "mode_t", "umode_t", "ctl_table"];
 const INCLUDED_FUNCTIONS: &[&str] = &[
@@ -55,56 +56,6 @@
     "xregs_state",
 ];
 
-fn handle_kernel_version_cfg(bindings_path: &PathBuf) {
-    let f = BufReader::new(fs::File::open(bindings_path).unwrap());
-    let mut version = None;
-    for line in f.lines() {
-        let line = line.unwrap();
-        if let Some(type_and_value) = line.split("pub const LINUX_VERSION_CODE").nth(1) {
-            if let Some(value) = type_and_value.split('=').nth(1) {
-                let raw_version = value.split(';').next().unwrap();
-                version = Some(raw_version.trim().parse::<u64>().unwrap());
-                break;
-            }
-        }
-    }
-    let version = version.expect("Couldn't find kernel version");
-    let (major, minor) = match version.to_be_bytes() {
-        [0, 0, 0, 0, 0, major, minor, _patch] => (major, minor),
-        _ => panic!("unable to parse LINUX_VERSION_CODE {:x}", version),
-    };
-
-    if major >= 6 {
-        panic!("Please update build.rs with the last 5.x version");
-        // Change this block to major >= 7, copy the below block for
-        // major >= 6, fill in unimplemented!() for major >= 5
-    }
-    if major >= 5 {
-        for x in 0..=if major > 5 { unimplemented!() } else { minor } {
-            println!("cargo:rustc-cfg=kernel_5_{}_0_or_greater", x);
-        }
-    }
-    if major >= 4 {
-        // We don't currently support anything older than 4.4
-        for x in 4..=if major > 4 { 20 } else { minor } {
-            println!("cargo:rustc-cfg=kernel_4_{}_0_or_greater", x);
-        }
-    }
-}
-
-fn handle_kernel_symbols_cfg(symvers_path: &PathBuf) {
-    let f = BufReader::new(fs::File::open(symvers_path).unwrap());
-    for line in f.lines() {
-        let line = line.unwrap();
-        if let Some(symbol) = line.split_ascii_whitespace().nth(1) {
-            if symbol == "setfl" {
-                println!("cargo:rustc-cfg=kernel_aufs_setfl");
-                break;
-            }
-        }
-    }
-}
-
 // Takes the CFLAGS from the kernel Makefile and changes all the include paths to be absolute
 // instead of relative.
 fn prepare_cflags(cflags: &str, kernel_dir: &str) -> Vec<String> {
@@ -112,6 +63,11 @@
     let mut cflag_iter = cflag_parts.iter();
     let mut kernel_args = vec![];
     while let Some(arg) = cflag_iter.next() {
+        // TODO: bindgen complains
+        if arg.starts_with("-Wp,-MMD") {
+            continue;
+        }
+
         if arg.starts_with("-I") && !arg.starts_with("-I/") {
             kernel_args.push(format!("-I{}/{}", kernel_dir, &arg[2..]));
         } else if arg == "-include" {
@@ -131,15 +87,12 @@
 
 fn main() {
     println!("cargo:rerun-if-env-changed=CC");
-    println!("cargo:rerun-if-env-changed=KDIR");
-    println!("cargo:rerun-if-env-changed=c_flags");
+    println!("cargo:rerun-if-env-changed=RUST_BINDGEN_CFLAGS");
 
-    let kernel_dir = env::var("KDIR").expect("Must be invoked from kernel makefile");
-    let kernel_cflags = env::var("c_flags").expect("Add 'export c_flags' to Kbuild");
-    let kbuild_cflags_module =
-        env::var("KBUILD_CFLAGS_MODULE").expect("Must be invoked from kernel makefile");
+    let kernel_dir = "../../";
+    let cflags = env::var("RUST_BINDGEN_CFLAGS")
+        .expect("Must be invoked from kernel makefile");
 
-    let cflags = format!("{} {}", kernel_cflags, kbuild_cflags_module);
     let kernel_args = prepare_cflags(&cflags, &kernel_dir);
 
     let target = env::var("TARGET").unwrap();
@@ -173,22 +126,8 @@
     }
     let bindings = builder.generate().expect("Unable to generate bindings");
 
-    let out_path = PathBuf::from(env::var("OUT_DIR").unwrap());
+    let out_path = PathBuf::from("src/bindings_gen.rs");
     bindings
-        .write_to_file(out_path.join("bindings.rs"))
+        .write_to_file(out_path)
         .expect("Couldn't write bindings!");
-
-    handle_kernel_version_cfg(&out_path.join("bindings.rs"));
-    handle_kernel_symbols_cfg(&PathBuf::from(&kernel_dir).join("Module.symvers"));
-
-    let mut builder = cc::Build::new();
-    builder.compiler(env::var("CC").unwrap_or_else(|_| "clang".to_string()));
-    builder.target(&target);
-    builder.warnings(false);
-    println!("cargo:rerun-if-changed=src/helpers.c");
-    builder.file("src/helpers.c");
-    for arg in kernel_args.iter() {
-        builder.flag(&arg);
-    }
-    builder.compile("helpers");
 }

And the driver:

@@ -1,35 +1,33 @@
-#![no_std]
-
-extern crate alloc;
+// SPDX-License-Identifier: GPL-2.0
 
-use alloc::borrow::ToOwned;
-use alloc::string::String;
+#![no_std]
 
-use linux_kernel_module::println;
+use kernel::prelude::*;
 
-struct HelloWorldModule {
+struct RustExample {
     message: String,
 }
 
-impl linux_kernel_module::KernelModule for HelloWorldModule {
-    fn init() -> linux_kernel_module::KernelResult<Self> {
-        println!("Hello kernel module!");
-        Ok(HelloWorldModule {
+impl KernelModule for RustExample {
+    fn init() -> KernelResult<Self> {
+        println!("Rust Example (init)");
+        println!("Am I built-in? {}", !cfg!(module));
+        Ok(RustExample {
             message: "on the heap!".to_owned(),
         })
     }
 }
 
-impl Drop for HelloWorldModule {
+impl Drop for RustExample {
     fn drop(&mut self) {
         println!("My message is {}", self.message);
-        println!("Goodbye kernel module!");
+        println!("Rust Example (exit)");
     }
 }
 
-linux_kernel_module::kernel_module!(
-    HelloWorldModule,
-    author: b"Fish in a Barrel Contributors",
-    description: b"An extremely simple kernel module",
-    license: b"GPL"
+kernel_module!(
+    RustExample,
+    author: b"Rust for Linux Contributors",
+    description: b"An example kernel module written in Rust",
+    license: b"GPL v2"
 );

This is awesome!

I'm going to try to split this out into patches mostly so it's easier for me to review and also because I suspect it'll be better for submission. There might also be some changes we can "upstream" into linux-kernel-module-rust.

I had the Kbuild stuff on my mind because of fishinabarrel/linux-kernel-module-rust#266 - do your Kbudild rulesx make .cmd files for us? If so, I might try to roll this approach back into our out-of-tree Makefiles....

Agree re credit, I assume it'll be easy to stick some Co-authored-by lines or whatever, I don't really have a strong opinion about whose name is on it :)

You'll probably want to cherry-pick fishinabarrel/linux-kernel-module-rust@abc9791 which further reduces the need for nightly features.

A few more comments

This is awesome!

Thanks!

I'm going to try to split this out into patches mostly so it's easier for me to review and also because I suspect it'll be better for submission.

Please note that this repo/branch/commits is only meant for development within GitHub -- later I'll have to squash everything on my side and prepare the proper PR anyway (see #1). So all these dev-commits/messages/merges will go away.

There might also be some changes we can "upstream" into linux-kernel-module-rust.

Not much (I intentionally modified as few things as possible in your original files). In particular:

• The build.rs things I removed are still needed on your side because you support several kernel versions etc.
• The example driver changes are intended to fit the new structure etc., so those aren't needed on your side.
• The built-in modules support is not used for external out-of-tree modules.
• The kbuild changes require modifying kbuild itself which you can't do.

Things that I think you could pick, however, are:

• The prelude idea.
• The SPDX lines (it is good to teach people convention even if they are out-of-tree).
• Building of helpers.c using kbuild rather than inside build.rs.

I should have probably left the prelude and SPDX changes for another PR to make it even easier to diff; I wasn't consistent on the strategy there.

I had the Kbuild stuff on my mind because of fishinabarrel/linux-kernel-module-rust#266. do your Kbudild rulesx make .cmd files for us? If so, I might try to roll this approach back into our out-of-tree Makefiles....

Kbuild files are intended for out-of-tree modules. Inside the kernel we just call it the Makefile, but it is still all kbuild (i.e. the build system)..cmd files are generated for the usual things kbuild does it currently (e.g. clang, ld).

What I did is teach kbuild (in a hacky way at some points) how to handle our Rust stuff in a way that adding new Rust modules looks like any other C module and as easy to use as possible, i.e. you only need to write:

# SPDX-License-Identifier: GPL-2.0

obj-$(CONFIG_RUST_EXAMPLE) += rust_example.o

Agree re credit, I assume it'll be easy to stick some Co-authored-by lines or whatever,

Yeah, exactly. Co-developed-by and Signed-off-by is the usual way.

FWIW, I managed to emulate the module_init codegen for builtin modules with something like this:

#![feature(global_asm)]

global_asm!(
    r#".section ".initcall6.init", "a"
    __initcall_foo6:
        .long   foo - .
        .previous
    "#
);

#[no_mangle]
fn foo() -> i32 {
    4
}

Converting to a macro that's parameterized on the function name is left as an exercise to the reader :-)

I am starting to wonder whether trying to emulate all the C module support is too much duplication. Perhaps we should generate and compile a C file for each defined Rust module on the fly (without the module writer having to write anything different).

Anyway, for the moment I added support to do conditional compilation on the kernel configuration variables (which is anyway good to have!) and used that to fully emulate the macro. That frees us from the arch/ change which was a blocker.

It has been a week so I am merging this so that we can start sending PRs etc.

Sorry, guys, I'm slammed at work. Hopefully I can help out more in the future; just excessively busy right now... :(

Is cargo pulling stuff from the network? Is there a way to make this work with only local crates, i.e. installed by the distro or pre-installed by the user prior to attempting to build the kernel?

Sorry, guys, I'm slammed at work. Hopefully I can help out more in the future; just excessively busy right now... :(

No worries!

Is cargo pulling stuff from the network? Is there a way to make this work with only local crates, i.e. installed by the distro or pre-installed by the user prior to attempting to build the kernel?

Yeah, it does. For the moment, it is downloading them, although the versions are locked (i.e. no surprise updates, which means no unknown/arbitrary code downloaded when running make). Having said that:

• It is possible to pre-download them by cargo fetch (I mention it in Documentation/rust/quick-start.rst).
• It is possible to require to pre-download them. This means failing the build otherwise. Harsh, but this would keep the expectation that no network connections are made while building, which I suspect will be a popular option. In a way, those dependencies are a "tool", like rustc and cargo themselves, that should be available, so it makes sense to require it.

To recap the situation of the dependencies for everyone: we can make it work with local code (the rust/shlex I added is an example of that), but there are 2 big ones that are harder to manage:

• Two compiler dependencies: it is not clear to me if newer/older rustcs will always be able to compile those. If not, it might mean we have to fix the version of the compiler, which is not great. In theory we could have a fork or our own version of those and make it work with any support compiler version, but I think for the moment it would be going too far (e.g. in the future we might want to have our own tailored alloc).
• The bindings generator: it uses a lot of dependencies, but we could either commit the generated code (so only needed if updating it -- the best option, except at the beginning people won't be happy to have to run this if they change some kernel API that we use) or save all of them as third-party code (not great, but at least they are not tied to the compiler, so it should never break).

Another topic is avoiding the connections to crates.io and github.com. For that, we could propose to host mirrors at kernel.org.

For crate dependencies, we should vendor those into the kernel tree under a specific path, and then configure that path as a directory source, along with passing the appropriate cargo options to never touch the network.

For bindgen/cbindgen, I'm wondering if we can get distributions to package it, and then just expect people to have it installed like cargo and rustc.

https://packages.debian.org/sid/bindgen :-)

…

On Thu, Sep 17, 2020 at 10:17 AM Josh Triplett ***@***.***> wrote: For crate dependencies, we should vendor those into the kernel tree under a specific path, and then configure that path as a directory registry, along with passing the appropriate cargo options to never touch the network. For bindgen/cbindgen, I'm wondering if we can get distributions to package it, and then just expect people to have it installed like cargo and rustc. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAAGBHXHEW7TYMV6OQUQ53SGIK7PANCNFSM4QZPGBCQ> .

-- All that is necessary for evil to succeed is for good people to do nothing.