[PATCH v2 1/3] kbuild: rust: add `CONFIG_RUSTC_CLANG_LLVM_COMPATIBLE`

[PATCH v2 1/3] kbuild: rust: add `CONFIG_RUSTC_CLANG_LLVM_COMPATIBLE`

[Gary Guo]

This config detects if Rust and Clang have matching LLVM major version.
All IR or bitcode operations (e.g. LTO) rely on LLVM major version to be
matching, otherwise it may generate errors, or worse, miscompile silently
due to change of IR semantics.

It's usually suggested to use the exact same LLVM version, but this can
be difficult to guarantee. Rust's suggestion [1] is also major-version only,
so I think this check is sufficient for the kernel.

Link: https://doc.rust-lang.org/rustc/linker-plugin-lto.html [1]
Signed-off-by: Gary Guo <gary@garyguo.net>
---
 init/Kconfig | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/init/Kconfig b/init/Kconfig
index a20e6efd3f0f..cbe4b2922ce2 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -82,6 +82,21 @@ config RUSTC_LLVM_VERSION
 	int
 	default $(rustc-llvm-version)
 
+config RUSTC_LLVM_MAJOR_VERSION
+	int
+	default $(shell,expr $(rustc-llvm-version) / 10000)
+
+config RUSTC_CLANG_LLVM_COMPATIBLE
+	bool
+	default y if CC_IS_CLANG && RUSTC_LLVM_MAJOR_VERSION = $(shell,expr $(cc-version) / 10000)
+	help
+	  This indicates whether Rust and Clang use LLVM of the same major
+	  version.
+
+	  Operations involving handling LLVM IR or bitcode (e.g. cross-language
+	  LTO) requires the same LLVM major version to work properly. For best
+	  compatible it is recommended that the exact same LLVM is used.
+
 config CC_CAN_LINK
 	bool
 	default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m64-flag)) if 64BIT
-- 
2.47.0

[PATCH v2 2/3] kbuild: rust: provide an option to inline C helpers into Rust

[Gary Guo]

A new Kconfig option, `RUST_INLINE_HELPERS` is added to allow C helpers
(which was created to allow Rust to call into inline/macro C functions
without having to re-implement the logic in Rust) to be inlined into
Rust crates without performing a global LTO.

If the option is enabled, the following is performed:
* For helpers, instead of compiling them to object file to be linked
  into vmlinux, we compile them to LLVM IR.
* The LLVM IR is patched to add `linkonce_odr` linkage. This linkage
  means that the function is inlineable (effect of `_odr`), and the
  symbols generated will have weak linkage if emitted into object file
  (important since as later described, we might have multiple copies of
  the same symbol) and it will may be discarded if it is not invoked or
  all invocations are inlined.
* The LLVM IR is compiled to bitcode (This is step is not necessary, but
  is a performance optimisation to prevent LLVM from always have to
  reparse the same IR).
* When a Rust crate is compiled, instead of generating object file, we
  ask LLVM bitcode to be generated.
* llvm-link is invoked to combine the helper bitcode with the crate
  bitcode. This step is similar to LTO, but this is much faster since it
  only needs to inline the helpers.
* clang is invoked to turn the combined bitcode into object file.

Some caveats with the option:
* clang and Rust doesn't have the exact target string. Manual inspection
  shows that they should be compatible, but since they are not exactly
  the same LLVM seems to prefer not inlining them. This is bypassed with
  `--ignore-tti-inline-compatible`.
* LLVM doesn't want to inline functions combined with
  `-fno-delete-null-pointer-checks` with code compiled without. So we
  remove this command when compiling helpers. I think this should be
  okay since this is one of the hardening features and we shouldn't have
  null pointer dereferences in these helpers.

The checks can also be bypassed with force inlining (`__always_inline`)
but the behaviour is the same with extra options.

Co-developed-by: Boqun Feng <boqun.feng@gmail.com>
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
Signed-off-by: Gary Guo <gary@garyguo.net>
---
 Makefile                     |  4 +++-
 lib/Kconfig.debug            | 14 ++++++++++++++
 rust/Makefile                | 31 +++++++++++++++++++++++++++----
 rust/exports.c               |  5 ++++-
 rust/helpers/blk.c           |  5 +++--
 rust/helpers/bug.c           |  3 ++-
 rust/helpers/build_bug.c     |  3 ++-
 rust/helpers/cred.c          |  5 +++--
 rust/helpers/err.c           |  7 ++++---
 rust/helpers/fs.c            |  3 ++-
 rust/helpers/helpers.h       | 12 ++++++++++++
 rust/helpers/jump_label.c    |  3 ++-
 rust/helpers/kunit.c         |  3 ++-
 rust/helpers/mutex.c         |  7 ++++---
 rust/helpers/page.c          |  8 +++++---
 rust/helpers/pid_namespace.c |  7 ++++---
 rust/helpers/rbtree.c        |  6 ++++--
 rust/helpers/refcount.c      |  7 ++++---
 rust/helpers/security.c      |  7 ++++---
 rust/helpers/signal.c        |  3 ++-
 rust/helpers/slab.c          |  5 +++--
 rust/helpers/spinlock.c      | 11 ++++++-----
 rust/helpers/task.c          | 23 ++++++++++++-----------
 rust/helpers/uaccess.c       |  9 +++++----
 rust/helpers/vmalloc.c       |  3 ++-
 rust/helpers/wait.c          |  3 ++-
 rust/helpers/workqueue.c     |  9 ++++++---
 scripts/Makefile.build       |  5 ++++-
 28 files changed, 147 insertions(+), 64 deletions(-)
 create mode 100644 rust/helpers/helpers.h

diff --git a/Makefile b/Makefile
index e5b8a8832c0c..5fbb5e829f81 100644
--- a/Makefile
+++ b/Makefile
@@ -509,6 +509,8 @@ OBJCOPY		= $(LLVM_PREFIX)llvm-objcopy$(LLVM_SUFFIX)
 OBJDUMP		= $(LLVM_PREFIX)llvm-objdump$(LLVM_SUFFIX)
 READELF		= $(LLVM_PREFIX)llvm-readelf$(LLVM_SUFFIX)
 STRIP		= $(LLVM_PREFIX)llvm-strip$(LLVM_SUFFIX)
+LLVM_LINK	= $(LLVM_PREFIX)llvm-link$(LLVM_SUFFIX)
+LLVM_AS	= $(LLVM_PREFIX)llvm-as$(LLVM_SUFFIX)
 else
 CC		= $(CROSS_COMPILE)gcc
 LD		= $(CROSS_COMPILE)ld
@@ -616,7 +618,7 @@ export RUSTC_BOOTSTRAP := 1
 export CLIPPY_CONF_DIR := $(srctree)
 
 export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE LD CC HOSTPKG_CONFIG
-export RUSTC RUSTDOC RUSTFMT RUSTC_OR_CLIPPY_QUIET RUSTC_OR_CLIPPY BINDGEN
+export RUSTC RUSTDOC RUSTFMT RUSTC_OR_CLIPPY_QUIET RUSTC_OR_CLIPPY BINDGEN LLVM_LINK LLVM_AS
 export HOSTRUSTC KBUILD_HOSTRUSTFLAGS
 export CPP AR NM STRIP OBJCOPY OBJDUMP READELF PAHOLE RESOLVE_BTFIDS LEX YACC AWK INSTALLKERNEL
 export PERL PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index f3d723705879..8a8834128f13 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -3253,6 +3253,20 @@ config RUST_KERNEL_DOCTESTS
 
 	  If unsure, say N.
 
+config RUST_INLINE_HELPERS
+    bool "Inline C helpers into Rust crates"
+    depends on RUST && RUSTC_CLANG_LLVM_COMPATIBLE
+    help
+        Links C helpers into with Rust crates through LLVM IR.
+
+        If this option is enabled, instead of generating object files directly,
+        rustc is asked to produce LLVM IR, which is then linked together with
+        the LLVM IR of C helpers, before object file is generated.
+
+        This requires a matching LLVM version for Clang and rustc.
+
+        If unsure, say N.
+
 endmenu # "Rust"
 
 endmenu # Kernel hacking
diff --git a/rust/Makefile b/rust/Makefile
index a40a3936126d..8a3d61e5a799 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -6,9 +6,14 @@ rustdoc_output := $(objtree)/Documentation/output/rust/rustdoc
 obj-$(CONFIG_RUST) += core.o compiler_builtins.o ffi.o
 always-$(CONFIG_RUST) += exports_core_generated.h
 
+ifdef CONFIG_RUST_INLINE_HELPERS
+always-$(CONFIG_RUST) += helpers/helpers.bc
+else
+obj-$(CONFIG_RUST) += helpers/helpers.o
+always-$(CONFIG_RUST) += exports_helpers_generated.h
+endif
 # Missing prototypes are expected in the helpers since these are exported
 # for Rust only, thus there is no header nor prototypes.
-obj-$(CONFIG_RUST) += helpers/helpers.o
 CFLAGS_REMOVE_helpers/helpers.o = -Wmissing-prototypes -Wmissing-declarations
 
 always-$(CONFIG_RUST) += libmacros.so
@@ -16,8 +21,7 @@ no-clean-files += libmacros.so
 
 always-$(CONFIG_RUST) += bindings/bindings_generated.rs bindings/bindings_helpers_generated.rs
 obj-$(CONFIG_RUST) += bindings.o kernel.o
-always-$(CONFIG_RUST) += exports_helpers_generated.h \
-    exports_bindings_generated.h exports_kernel_generated.h
+always-$(CONFIG_RUST) += exports_bindings_generated.h exports_kernel_generated.h
 
 always-$(CONFIG_RUST) += uapi/uapi_generated.rs
 obj-$(CONFIG_RUST) += uapi.o
@@ -329,6 +333,18 @@ $(obj)/bindings/bindings_helpers_generated.rs: private bindgen_target_extra = ;
 $(obj)/bindings/bindings_helpers_generated.rs: $(src)/helpers/helpers.c FORCE
 	$(call if_changed_dep,bindgen)
 
+# When compiling helpers, filter `-fno-delete-null-pointer-checks` since LLVM
+# prevents inlining such functions to be inlined into functions compiled
+# without the option (e.g. Rust functions).
+quiet_cmd_rust_helper = HELPER  $@
+      cmd_rust_helper = \
+	$(CC) $(filter-out -fno-delete-null-pointer-checks $(CFLAGS_REMOVE_helpers/helpers.o), $(c_flags)) -S $< -emit-llvm -o $(patsubst %.bc,%.ll,$@); \
+	sed -i 's/^define dso_local/define linkonce_odr dso_local/g' $(patsubst %.bc,%.ll,$@); \
+	$(LLVM_AS) $(patsubst %.bc,%.ll,$@) -o $@
+
+$(obj)/helpers/helpers.bc: $(obj)/helpers/helpers.c FORCE
+	+$(call if_changed_dep,rust_helper)
+
 quiet_cmd_exports = EXPORTS $@
       cmd_exports = \
 	$(NM) -p --defined-only $< \
@@ -373,11 +389,13 @@ quiet_cmd_rustc_library = $(if $(skip_clippy),RUSTC,$(RUSTC_OR_CLIPPY_QUIET)) L
 	OBJTREE=$(abspath $(objtree)) \
 	$(if $(skip_clippy),$(RUSTC),$(RUSTC_OR_CLIPPY)) \
 		$(filter-out $(skip_flags),$(rust_flags) $(rustc_target_flags)) \
-		--emit=dep-info=$(depfile) --emit=obj=$@ \
+		--emit=dep-info=$(depfile) --emit=$(if $(link_helper),llvm-bc=$(patsubst %.o,%.bc,$@),obj=$@) \
 		--emit=metadata=$(dir $@)$(patsubst %.o,lib%.rmeta,$(notdir $@)) \
 		--crate-type rlib -L$(objtree)/$(obj) \
 		--crate-name $(patsubst %.o,%,$(notdir $@)) $< \
 		--sysroot=/dev/null \
+	$(if $(link_helper),;$(LLVM_LINK) $(patsubst %.o,%.bc,$@) $(obj)/helpers/helpers.bc -o $(patsubst %.o,%.m.bc,$@); \
+		$(CC) $(CLANG_FLAGS) $(KBUILD_CFLAGS) -mllvm=--ignore-tti-inline-compatible -c $(patsubst %.o,%.m.bc,$@) -o $@) \
 	$(if $(rustc_objcopy),;$(OBJCOPY) $(rustc_objcopy) $@) \
 	$(cmd_objtool)
 
@@ -450,4 +468,9 @@ ifdef CONFIG_JUMP_LABEL
 $(obj)/kernel.o: $(obj)/kernel/generated_arch_static_branch_asm.rs
 endif
 
+ifdef CONFIG_RUST_INLINE_HELPERS
+$(obj)/kernel.o: private link_helper = 1
+$(obj)/kernel.o: $(obj)/helpers/helpers.bc
+endif
+
 endif # CONFIG_RUST
diff --git a/rust/exports.c b/rust/exports.c
index 587f0e776aba..1b52460b0f4e 100644
--- a/rust/exports.c
+++ b/rust/exports.c
@@ -16,10 +16,13 @@
 #define EXPORT_SYMBOL_RUST_GPL(sym) extern int sym; EXPORT_SYMBOL_GPL(sym)
 
 #include "exports_core_generated.h"
-#include "exports_helpers_generated.h"
 #include "exports_bindings_generated.h"
 #include "exports_kernel_generated.h"
 
+#ifndef CONFIG_RUST_INLINE_HELPERS
+#include "exports_helpers_generated.h"
+#endif
+
 // For modules using `rust/build_error.rs`.
 #ifdef CONFIG_RUST_BUILD_ASSERT_ALLOW
 EXPORT_SYMBOL_RUST_GPL(rust_build_error);
diff --git a/rust/helpers/blk.c b/rust/helpers/blk.c
index cc9f4e6a2d23..a96e5ddf384d 100644
--- a/rust/helpers/blk.c
+++ b/rust/helpers/blk.c
@@ -2,13 +2,14 @@
 
 #include <linux/blk-mq.h>
 #include <linux/blkdev.h>
+#include "helpers.h"
 
-void *rust_helper_blk_mq_rq_to_pdu(struct request *rq)
+__rust_helper void *rust_helper_blk_mq_rq_to_pdu(struct request *rq)
 {
 	return blk_mq_rq_to_pdu(rq);
 }
 
-struct request *rust_helper_blk_mq_rq_from_pdu(void *pdu)
+__rust_helper struct request *rust_helper_blk_mq_rq_from_pdu(void *pdu)
 {
 	return blk_mq_rq_from_pdu(pdu);
 }
diff --git a/rust/helpers/bug.c b/rust/helpers/bug.c
index e2d13babc737..3bff5b730ca8 100644
--- a/rust/helpers/bug.c
+++ b/rust/helpers/bug.c
@@ -1,8 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <linux/bug.h>
+#include "helpers.h"
 
-__noreturn void rust_helper_BUG(void)
+__rust_helper __noreturn void rust_helper_BUG(void)
 {
 	BUG();
 }
diff --git a/rust/helpers/build_bug.c b/rust/helpers/build_bug.c
index 44e579488037..9dc273fd8db1 100644
--- a/rust/helpers/build_bug.c
+++ b/rust/helpers/build_bug.c
@@ -1,8 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <linux/errname.h>
+#include "helpers.h"
 
-const char *rust_helper_errname(int err)
+__rust_helper const char *rust_helper_errname(int err)
 {
 	return errname(err);
 }
diff --git a/rust/helpers/cred.c b/rust/helpers/cred.c
index fde7ae20cdd1..9dceeb7f8c87 100644
--- a/rust/helpers/cred.c
+++ b/rust/helpers/cred.c
@@ -1,13 +1,14 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <linux/cred.h>
+#include "helpers.h"
 
-const struct cred *rust_helper_get_cred(const struct cred *cred)
+__rust_helper const struct cred *rust_helper_get_cred(const struct cred *cred)
 {
 	return get_cred(cred);
 }
 
-void rust_helper_put_cred(const struct cred *cred)
+__rust_helper void rust_helper_put_cred(const struct cred *cred)
 {
 	put_cred(cred);
 }
diff --git a/rust/helpers/err.c b/rust/helpers/err.c
index 544c7cb86632..b05516fce504 100644
--- a/rust/helpers/err.c
+++ b/rust/helpers/err.c
@@ -1,18 +1,19 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <linux/err.h>
+#include "helpers.h"
 
-__force void *rust_helper_ERR_PTR(long err)
+__rust_helper __force void *rust_helper_ERR_PTR(long err)
 {
 	return ERR_PTR(err);
 }
 
-bool rust_helper_IS_ERR(__force const void *ptr)
+__rust_helper bool rust_helper_IS_ERR(__force const void *ptr)
 {
 	return IS_ERR(ptr);
 }
 
-long rust_helper_PTR_ERR(__force const void *ptr)
+__rust_helper long rust_helper_PTR_ERR(__force const void *ptr)
 {
 	return PTR_ERR(ptr);
 }
diff --git a/rust/helpers/fs.c b/rust/helpers/fs.c
index a75c96763372..7d44bda94203 100644
--- a/rust/helpers/fs.c
+++ b/rust/helpers/fs.c
@@ -5,8 +5,9 @@
  */
 
 #include <linux/fs.h>
+#include "helpers.h"
 
-struct file *rust_helper_get_file(struct file *f)
+__rust_helper struct file *rust_helper_get_file(struct file *f)
 {
 	return get_file(f);
 }
diff --git a/rust/helpers/helpers.h b/rust/helpers/helpers.h
new file mode 100644
index 000000000000..9248692146f3
--- /dev/null
+++ b/rust/helpers/helpers.h
@@ -0,0 +1,12 @@
+#ifndef RUST_HELPERS_H
+#define RUST_HELPERS_H
+
+#include <linux/compiler_types.h>
+
+#ifdef __BINDGEN__
+#define __rust_helper
+#else
+#define __rust_helper inline
+#endif
+
+#endif
diff --git a/rust/helpers/jump_label.c b/rust/helpers/jump_label.c
index fc1f1e0df08e..c97a66c51cce 100644
--- a/rust/helpers/jump_label.c
+++ b/rust/helpers/jump_label.c
@@ -5,9 +5,10 @@
  */
 
 #include <linux/jump_label.h>
+#include "helpers.h"
 
 #ifndef CONFIG_JUMP_LABEL
-int rust_helper_static_key_count(struct static_key *key)
+__rust_helper int rust_helper_static_key_count(struct static_key *key)
 {
 	return static_key_count(key);
 }
diff --git a/rust/helpers/kunit.c b/rust/helpers/kunit.c
index b85a4d394c11..f3621a564626 100644
--- a/rust/helpers/kunit.c
+++ b/rust/helpers/kunit.c
@@ -1,8 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <kunit/test-bug.h>
+#include "helpers.h"
 
-struct kunit *rust_helper_kunit_get_current_test(void)
+__rust_helper struct kunit *rust_helper_kunit_get_current_test(void)
 {
 	return kunit_get_current_test();
 }
diff --git a/rust/helpers/mutex.c b/rust/helpers/mutex.c
index 7e00680958ef..6259b5c959f6 100644
--- a/rust/helpers/mutex.c
+++ b/rust/helpers/mutex.c
@@ -1,14 +1,15 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <linux/mutex.h>
+#include "helpers.h"
 
-void rust_helper_mutex_lock(struct mutex *lock)
+__rust_helper void rust_helper_mutex_lock(struct mutex *lock)
 {
 	mutex_lock(lock);
 }
 
-void rust_helper___mutex_init(struct mutex *mutex, const char *name,
-			      struct lock_class_key *key)
+__rust_helper void rust_helper___mutex_init(struct mutex *mutex, const char *name,
+					    struct lock_class_key *key)
 {
 	__mutex_init(mutex, name, key);
 }
diff --git a/rust/helpers/page.c b/rust/helpers/page.c
index b3f2b8fbf87f..cf7deea25cfa 100644
--- a/rust/helpers/page.c
+++ b/rust/helpers/page.c
@@ -2,18 +2,20 @@
 
 #include <linux/gfp.h>
 #include <linux/highmem.h>
+#include "helpers.h"
 
-struct page *rust_helper_alloc_pages(gfp_t gfp_mask, unsigned int order)
+__rust_helper struct page *rust_helper_alloc_pages(gfp_t gfp_mask,
+						   unsigned int order)
 {
 	return alloc_pages(gfp_mask, order);
 }
 
-void *rust_helper_kmap_local_page(struct page *page)
+__rust_helper void *rust_helper_kmap_local_page(struct page *page)
 {
 	return kmap_local_page(page);
 }
 
-void rust_helper_kunmap_local(const void *addr)
+__rust_helper void rust_helper_kunmap_local(const void *addr)
 {
 	kunmap_local(addr);
 }
diff --git a/rust/helpers/pid_namespace.c b/rust/helpers/pid_namespace.c
index f41482bdec9a..f066fa4e7872 100644
--- a/rust/helpers/pid_namespace.c
+++ b/rust/helpers/pid_namespace.c
@@ -2,19 +2,20 @@
 
 #include <linux/pid_namespace.h>
 #include <linux/cleanup.h>
+#include "helpers.h"
 
-struct pid_namespace *rust_helper_get_pid_ns(struct pid_namespace *ns)
+__rust_helper struct pid_namespace *rust_helper_get_pid_ns(struct pid_namespace *ns)
 {
 	return get_pid_ns(ns);
 }
 
-void rust_helper_put_pid_ns(struct pid_namespace *ns)
+__rust_helper void rust_helper_put_pid_ns(struct pid_namespace *ns)
 {
 	put_pid_ns(ns);
 }
 
 /* Get a reference on a task's pid namespace. */
-struct pid_namespace *rust_helper_task_get_pid_ns(struct task_struct *task)
+__rust_helper struct pid_namespace *rust_helper_task_get_pid_ns(struct task_struct *task)
 {
 	struct pid_namespace *pid_ns;
 
diff --git a/rust/helpers/rbtree.c b/rust/helpers/rbtree.c
index 6d404b84a9b5..784d8796aa1c 100644
--- a/rust/helpers/rbtree.c
+++ b/rust/helpers/rbtree.c
@@ -1,9 +1,11 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <linux/rbtree.h>
+#include "helpers.h"
 
-void rust_helper_rb_link_node(struct rb_node *node, struct rb_node *parent,
-			      struct rb_node **rb_link)
+__rust_helper void rust_helper_rb_link_node(struct rb_node *node,
+					    struct rb_node *parent,
+					    struct rb_node **rb_link)
 {
 	rb_link_node(node, parent, rb_link);
 }
diff --git a/rust/helpers/refcount.c b/rust/helpers/refcount.c
index d6adbd2e45a1..ad80e153bf37 100644
--- a/rust/helpers/refcount.c
+++ b/rust/helpers/refcount.c
@@ -1,18 +1,19 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <linux/refcount.h>
+#include "helpers.h"
 
-refcount_t rust_helper_REFCOUNT_INIT(int n)
+__rust_helper refcount_t rust_helper_REFCOUNT_INIT(int n)
 {
 	return (refcount_t)REFCOUNT_INIT(n);
 }
 
-void rust_helper_refcount_inc(refcount_t *r)
+__rust_helper void rust_helper_refcount_inc(refcount_t *r)
 {
 	refcount_inc(r);
 }
 
-bool rust_helper_refcount_dec_and_test(refcount_t *r)
+__rust_helper bool rust_helper_refcount_dec_and_test(refcount_t *r)
 {
 	return refcount_dec_and_test(r);
 }
diff --git a/rust/helpers/security.c b/rust/helpers/security.c
index 239e5b4745fe..b0c14e1b3aff 100644
--- a/rust/helpers/security.c
+++ b/rust/helpers/security.c
@@ -1,19 +1,20 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <linux/security.h>
+#include "helpers.h"
 
 #ifndef CONFIG_SECURITY
-void rust_helper_security_cred_getsecid(const struct cred *c, u32 *secid)
+__rust_helper void rust_helper_security_cred_getsecid(const struct cred *c, u32 *secid)
 {
 	security_cred_getsecid(c, secid);
 }
 
-int rust_helper_security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
+__rust_helper int rust_helper_security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
 {
 	return security_secid_to_secctx(secid, secdata, seclen);
 }
 
-void rust_helper_security_release_secctx(char *secdata, u32 seclen)
+__rust_helper void rust_helper_security_release_secctx(char *secdata, u32 seclen)
 {
 	security_release_secctx(secdata, seclen);
 }
diff --git a/rust/helpers/signal.c b/rust/helpers/signal.c
index 1a6bbe9438e2..67d3fe8d5132 100644
--- a/rust/helpers/signal.c
+++ b/rust/helpers/signal.c
@@ -1,8 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <linux/sched/signal.h>
+#include "helpers.h"
 
-int rust_helper_signal_pending(struct task_struct *t)
+__rust_helper int rust_helper_signal_pending(struct task_struct *t)
 {
 	return signal_pending(t);
 }
diff --git a/rust/helpers/slab.c b/rust/helpers/slab.c
index a842bfbddcba..f8776884b729 100644
--- a/rust/helpers/slab.c
+++ b/rust/helpers/slab.c
@@ -1,14 +1,15 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <linux/slab.h>
+#include "helpers.h"
 
-void * __must_check __realloc_size(2)
+__rust_helper void *__must_check __realloc_size(2)
 rust_helper_krealloc(const void *objp, size_t new_size, gfp_t flags)
 {
 	return krealloc(objp, new_size, flags);
 }
 
-void * __must_check __realloc_size(2)
+__rust_helper void * __must_check __realloc_size(2)
 rust_helper_kvrealloc(const void *p, size_t size, gfp_t flags)
 {
 	return kvrealloc(p, size, flags);
diff --git a/rust/helpers/spinlock.c b/rust/helpers/spinlock.c
index 5971fdf6f755..567161c19a16 100644
--- a/rust/helpers/spinlock.c
+++ b/rust/helpers/spinlock.c
@@ -1,9 +1,10 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <linux/spinlock.h>
+#include "helpers.h"
 
-void rust_helper___spin_lock_init(spinlock_t *lock, const char *name,
-				  struct lock_class_key *key)
+__rust_helper void rust_helper___spin_lock_init(spinlock_t *lock, const char *name,
+						struct lock_class_key *key)
 {
 #ifdef CONFIG_DEBUG_SPINLOCK
 # if defined(CONFIG_PREEMPT_RT)
@@ -16,17 +17,17 @@ void rust_helper___spin_lock_init(spinlock_t *lock, const char *name,
 #endif /* CONFIG_DEBUG_SPINLOCK */
 }
 
-void rust_helper_spin_lock(spinlock_t *lock)
+__rust_helper void rust_helper_spin_lock(spinlock_t *lock)
 {
 	spin_lock(lock);
 }
 
-void rust_helper_spin_unlock(spinlock_t *lock)
+__rust_helper void rust_helper_spin_unlock(spinlock_t *lock)
 {
 	spin_unlock(lock);
 }
 
-int rust_helper_spin_trylock(spinlock_t *lock)
+__rust_helper int rust_helper_spin_trylock(spinlock_t *lock)
 {
 	return spin_trylock(lock);
 }
diff --git a/rust/helpers/task.c b/rust/helpers/task.c
index 31c33ea2dce6..741e1a4b15ee 100644
--- a/rust/helpers/task.c
+++ b/rust/helpers/task.c
@@ -1,56 +1,57 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <linux/sched/task.h>
+#include "helpers.h"
 
-struct task_struct *rust_helper_get_current(void)
+__rust_helper struct task_struct *rust_helper_get_current(void)
 {
 	return current;
 }
 
-void rust_helper_get_task_struct(struct task_struct *t)
+__rust_helper void rust_helper_get_task_struct(struct task_struct *t)
 {
 	get_task_struct(t);
 }
 
-void rust_helper_put_task_struct(struct task_struct *t)
+__rust_helper void rust_helper_put_task_struct(struct task_struct *t)
 {
 	put_task_struct(t);
 }
 
-kuid_t rust_helper_task_uid(struct task_struct *task)
+__rust_helper kuid_t rust_helper_task_uid(struct task_struct *task)
 {
 	return task_uid(task);
 }
 
-kuid_t rust_helper_task_euid(struct task_struct *task)
+__rust_helper kuid_t rust_helper_task_euid(struct task_struct *task)
 {
 	return task_euid(task);
 }
 
 #ifndef CONFIG_USER_NS
-uid_t rust_helper_from_kuid(struct user_namespace *to, kuid_t uid)
+__rust_helper uid_t rust_helper_from_kuid(struct user_namespace *to, kuid_t uid)
 {
 	return from_kuid(to, uid);
 }
 #endif /* CONFIG_USER_NS */
 
-bool rust_helper_uid_eq(kuid_t left, kuid_t right)
+__rust_helper bool rust_helper_uid_eq(kuid_t left, kuid_t right)
 {
 	return uid_eq(left, right);
 }
 
-kuid_t rust_helper_current_euid(void)
+__rust_helper kuid_t rust_helper_current_euid(void)
 {
 	return current_euid();
 }
 
-struct user_namespace *rust_helper_current_user_ns(void)
+__rust_helper struct user_namespace *rust_helper_current_user_ns(void)
 {
 	return current_user_ns();
 }
 
-pid_t rust_helper_task_tgid_nr_ns(struct task_struct *tsk,
-				  struct pid_namespace *ns)
+__rust_helper pid_t rust_helper_task_tgid_nr_ns(struct task_struct *tsk,
+						struct pid_namespace *ns)
 {
 	return task_tgid_nr_ns(tsk, ns);
 }
diff --git a/rust/helpers/uaccess.c b/rust/helpers/uaccess.c
index f49076f813cd..08bf1851c06a 100644
--- a/rust/helpers/uaccess.c
+++ b/rust/helpers/uaccess.c
@@ -1,15 +1,16 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <linux/uaccess.h>
+#include "helpers.h"
 
-unsigned long rust_helper_copy_from_user(void *to, const void __user *from,
-					 unsigned long n)
+__rust_helper unsigned long
+rust_helper_copy_from_user(void *to, const void __user *from, unsigned long n)
 {
 	return copy_from_user(to, from, n);
 }
 
-unsigned long rust_helper_copy_to_user(void __user *to, const void *from,
-				       unsigned long n)
+__rust_helper unsigned long
+rust_helper_copy_to_user(void __user *to, const void *from, unsigned long n)
 {
 	return copy_to_user(to, from, n);
 }
diff --git a/rust/helpers/vmalloc.c b/rust/helpers/vmalloc.c
index 80d34501bbc0..6d258baa4c9a 100644
--- a/rust/helpers/vmalloc.c
+++ b/rust/helpers/vmalloc.c
@@ -1,8 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <linux/vmalloc.h>
+#include "helpers.h"
 
-void * __must_check __realloc_size(2)
+__rust_helper void * __must_check __realloc_size(2)
 rust_helper_vrealloc(const void *p, size_t size, gfp_t flags)
 {
 	return vrealloc(p, size, flags);
diff --git a/rust/helpers/wait.c b/rust/helpers/wait.c
index ae48e33d9da3..7beaa85494bc 100644
--- a/rust/helpers/wait.c
+++ b/rust/helpers/wait.c
@@ -1,8 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <linux/wait.h>
+#include "helpers.h"
 
-void rust_helper_init_wait(struct wait_queue_entry *wq_entry)
+__rust_helper void rust_helper_init_wait(struct wait_queue_entry *wq_entry)
 {
 	init_wait(wq_entry);
 }
diff --git a/rust/helpers/workqueue.c b/rust/helpers/workqueue.c
index b2b82753509b..4256f8c91ce6 100644
--- a/rust/helpers/workqueue.c
+++ b/rust/helpers/workqueue.c
@@ -1,10 +1,13 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <linux/workqueue.h>
+#include "helpers.h"
 
-void rust_helper_init_work_with_key(struct work_struct *work, work_func_t func,
-				    bool onstack, const char *name,
-				    struct lock_class_key *key)
+__rust_helper void rust_helper_init_work_with_key(struct work_struct *work,
+						  work_func_t func,
+						  bool onstack,
+						  const char *name,
+						  struct lock_class_key *key)
 {
 	__init_work(work, onstack);
 	work->data = (atomic_long_t)WORK_DATA_INIT();
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index c16e4cf54d77..77c204449d1b 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -239,7 +239,10 @@ rust_common_cmd = \
 # would not match each other.
 
 quiet_cmd_rustc_o_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
-      cmd_rustc_o_rs = $(rust_common_cmd) --emit=obj=$@ $< $(cmd_objtool)
+      cmd_rustc_o_rs = $(rust_common_cmd) --emit=$(if $(CONFIG_RUST_INLINE_HELPERS),llvm-bc=$(patsubst %.o,%.bc,$@),obj=$@) $< \
+	$(if $(CONFIG_RUST_INLINE_HELPERS),;$(LLVM_LINK) $(patsubst %.o,%.bc,$@) $(objtree)/rust/helpers/helpers.bc -o $(patsubst %.o,%.m.bc,$@); \
+		$(CC) $(CLANG_FLAGS) $(KBUILD_CFLAGS) -mllvm=--ignore-tti-inline-compatible -c $(patsubst %.o,%.m.bc,$@) -o $@) \
+	$(cmd_objtool)
 
 define rule_rustc_o_rs
 	$(call cmd_and_fixdep,rustc_o_rs)
-- 
2.47.0

[PATCH v2 3/3] rust: alloc: make `ReallocFunc::call` inline

[Gary Guo]

This function can be called with different function pointers when
different allocator (e.g. Kmalloc, Vmalloc, KVmalloc), however since
this function is not polymorphic, only one instance is generated,
and function pointers are used. Given that this function is called
for any Rust-side allocation/deallocation, performance matters a lot,
so making this function inlineable.

This is discovered when doing helper inlining work, since it's discovered
that even with helpers inlined, rust_helper_ symbols are still present
in final vmlinux binary, and it turns out this function is inhibiting
the inlining, and introducing indirect function calls.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 rust/kernel/alloc/allocator.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs
index 439985e29fbc..aa2dfa9dca4c 100644
--- a/rust/kernel/alloc/allocator.rs
+++ b/rust/kernel/alloc/allocator.rs
@@ -80,6 +80,7 @@ impl ReallocFunc {
     /// This method has the same guarantees as `Allocator::realloc`. Additionally
     /// - it accepts any pointer to a valid memory allocation allocated by this function.
     /// - memory allocated by this function remains valid until it is passed to this function.
+    #[inline]
     unsafe fn call(
         &self,
         ptr: Option<NonNull<u8>>,
-- 
2.47.0

Re: [PATCH v2 2/3] kbuild: rust: provide an option to inline C helpers into Rust

[Randy Dunlap]

On 1/5/25 11:40 AM, Gary Guo wrote:
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index f3d723705879..8a8834128f13 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -3253,6 +3253,20 @@ config RUST_KERNEL_DOCTESTS
>  
>  	  If unsure, say N.
>  
> +config RUST_INLINE_HELPERS
> +    bool "Inline C helpers into Rust crates"
> +    depends on RUST && RUSTC_CLANG_LLVM_COMPATIBLE
> +    help
> +        Links C helpers into with Rust crates through LLVM IR.

cannot parse "into with".

> +
> +        If this option is enabled, instead of generating object files directly,
> +        rustc is asked to produce LLVM IR, which is then linked together with
> +        the LLVM IR of C helpers, before object file is generated.
> +
> +        This requires a matching LLVM version for Clang and rustc.
> +
> +        If unsure, say N.
> +
>  endmenu # "Rust"

-- 
~Randy

Re: [PATCH v2 1/3] kbuild: rust: add `CONFIG_RUSTC_CLANG_LLVM_COMPATIBLE`

[Alice Ryhl]

On Sun, Jan 5, 2025 at 9:02 PM Gary Guo <gary@garyguo.net> wrote:
>
> This config detects if Rust and Clang have matching LLVM major version.
> All IR or bitcode operations (e.g. LTO) rely on LLVM major version to be
> matching, otherwise it may generate errors, or worse, miscompile silently
> due to change of IR semantics.
>
> It's usually suggested to use the exact same LLVM version, but this can
> be difficult to guarantee. Rust's suggestion [1] is also major-version only,
> so I think this check is sufficient for the kernel.
>
> Link: https://doc.rust-lang.org/rustc/linker-plugin-lto.html [1]
> Signed-off-by: Gary Guo <gary@garyguo.net>
> ---
>  init/Kconfig | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index a20e6efd3f0f..cbe4b2922ce2 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -82,6 +82,21 @@ config RUSTC_LLVM_VERSION
>         int
>         default $(rustc-llvm-version)
>
> +config RUSTC_LLVM_MAJOR_VERSION
> +       int
> +       default $(shell,expr $(rustc-llvm-version) / 10000)
> +
> +config RUSTC_CLANG_LLVM_COMPATIBLE
> +       bool
> +       default y if CC_IS_CLANG && RUSTC_LLVM_MAJOR_VERSION = $(shell,expr $(cc-version) / 10000)
> +       help
> +         This indicates whether Rust and Clang use LLVM of the same major
> +         version.
> +
> +         Operations involving handling LLVM IR or bitcode (e.g. cross-language
> +         LTO) requires the same LLVM major version to work properly. For best
> +         compatible it is recommended that the exact same LLVM is used.

typo: compatibility

Alice

Re: [PATCH v2 3/3] rust: alloc: make `ReallocFunc::call` inline

[Boqun Feng]

On Sun, Jan 05, 2025 at 07:40:06PM +0000, Gary Guo wrote:
> This function can be called with different function pointers when
> different allocator (e.g. Kmalloc, Vmalloc, KVmalloc), however since
> this function is not polymorphic, only one instance is generated,
> and function pointers are used. Given that this function is called
> for any Rust-side allocation/deallocation, performance matters a lot,
> so making this function inlineable.
> 
> This is discovered when doing helper inlining work, since it's discovered
> that even with helpers inlined, rust_helper_ symbols are still present
> in final vmlinux binary, and it turns out this function is inhibiting
> the inlining, and introducing indirect function calls.
> 
> Signed-off-by: Gary Guo <gary@garyguo.net>

Reviewed-by: Boqun Feng <boqun.feng@gmail.com>

This should go in even without the first 2 patches, right?

Regards,
Boqun

> ---
>  rust/kernel/alloc/allocator.rs | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs
> index 439985e29fbc..aa2dfa9dca4c 100644
> --- a/rust/kernel/alloc/allocator.rs
> +++ b/rust/kernel/alloc/allocator.rs
> @@ -80,6 +80,7 @@ impl ReallocFunc {
>      /// This method has the same guarantees as `Allocator::realloc`. Additionally
>      /// - it accepts any pointer to a valid memory allocation allocated by this function.
>      /// - memory allocated by this function remains valid until it is passed to this function.
> +    #[inline]
>      unsafe fn call(
>          &self,
>          ptr: Option<NonNull<u8>>,
> -- 
> 2.47.0
> 

Re: [PATCH v2 3/3] rust: alloc: make `ReallocFunc::call` inline

[Danilo Krummrich]

On Sun, Jan 05, 2025 at 07:40:06PM +0000, Gary Guo wrote:
> This function can be called with different function pointers when
> different allocator (e.g. Kmalloc, Vmalloc, KVmalloc), however since
> this function is not polymorphic, only one instance is generated,
> and function pointers are used. Given that this function is called
> for any Rust-side allocation/deallocation, performance matters a lot,
> so making this function inlineable.
> 
> This is discovered when doing helper inlining work, since it's discovered
> that even with helpers inlined, rust_helper_ symbols are still present
> in final vmlinux binary, and it turns out this function is inhibiting
> the inlining, and introducing indirect function calls.
> 
> Signed-off-by: Gary Guo <gary@garyguo.net>

Acked-by: Danilo Krummrich <dakr@kernel.org>

> ---
>  rust/kernel/alloc/allocator.rs | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs
> index 439985e29fbc..aa2dfa9dca4c 100644
> --- a/rust/kernel/alloc/allocator.rs
> +++ b/rust/kernel/alloc/allocator.rs
> @@ -80,6 +80,7 @@ impl ReallocFunc {
>      /// This method has the same guarantees as `Allocator::realloc`. Additionally
>      /// - it accepts any pointer to a valid memory allocation allocated by this function.
>      /// - memory allocated by this function remains valid until it is passed to this function.
> +    #[inline]
>      unsafe fn call(
>          &self,
>          ptr: Option<NonNull<u8>>,
> -- 
> 2.47.0
> 

Re: [PATCH v2 3/3] rust: alloc: make `ReallocFunc::call` inline

[Alice Ryhl]

On Sun, Jan 5, 2025 at 9:03 PM Gary Guo <gary@garyguo.net> wrote:
>
> This function can be called with different function pointers when
> different allocator (e.g. Kmalloc, Vmalloc, KVmalloc), however since
> this function is not polymorphic, only one instance is generated,
> and function pointers are used. Given that this function is called
> for any Rust-side allocation/deallocation, performance matters a lot,
> so making this function inlineable.
>
> This is discovered when doing helper inlining work, since it's discovered
> that even with helpers inlined, rust_helper_ symbols are still present
> in final vmlinux binary, and it turns out this function is inhibiting
> the inlining, and introducing indirect function calls.
>
> Signed-off-by: Gary Guo <gary@garyguo.net>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH v2 1/3] kbuild: rust: add `CONFIG_RUSTC_CLANG_LLVM_COMPATIBLE`

[Andreas Hindborg]

"Gary Guo" <gary@garyguo.net> writes:

> This config detects if Rust and Clang have matching LLVM major version.
> All IR or bitcode operations (e.g. LTO) rely on LLVM major version to be
> matching, otherwise it may generate errors, or worse, miscompile silently
> due to change of IR semantics.
>
> It's usually suggested to use the exact same LLVM version, but this can
> be difficult to guarantee. Rust's suggestion [1] is also major-version only,
> so I think this check is sufficient for the kernel.
>
> Link: https://doc.rust-lang.org/rustc/linker-plugin-lto.html [1]
> Signed-off-by: Gary Guo <gary@garyguo.net>


Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>


Best regards,
Andreas Hindborg

Re: [PATCH v2 2/3] kbuild: rust: provide an option to inline C helpers into Rust

[Andreas Hindborg]

"Gary Guo" <gary@garyguo.net> writes:

> A new Kconfig option, `RUST_INLINE_HELPERS` is added to allow C helpers
> (which was created to allow Rust to call into inline/macro C functions
> without having to re-implement the logic in Rust) to be inlined into
> Rust crates without performing a global LTO.
>
> If the option is enabled, the following is performed:
> * For helpers, instead of compiling them to object file to be linked
>   into vmlinux, we compile them to LLVM IR.
> * The LLVM IR is patched to add `linkonce_odr` linkage. This linkage
>   means that the function is inlineable (effect of `_odr`), and the
>   symbols generated will have weak linkage if emitted into object file
>   (important since as later described, we might have multiple copies of
>   the same symbol) and it will may be discarded if it is not invoked or
>   all invocations are inlined.
> * The LLVM IR is compiled to bitcode (This is step is not necessary, but
>   is a performance optimisation to prevent LLVM from always have to
>   reparse the same IR).
> * When a Rust crate is compiled, instead of generating object file, we
>   ask LLVM bitcode to be generated.
> * llvm-link is invoked to combine the helper bitcode with the crate
>   bitcode. This step is similar to LTO, but this is much faster since it
>   only needs to inline the helpers.
> * clang is invoked to turn the combined bitcode into object file.
>
> Some caveats with the option:
> * clang and Rust doesn't have the exact target string. Manual inspection
>   shows that they should be compatible, but since they are not exactly
>   the same LLVM seems to prefer not inlining them. This is bypassed with
>   `--ignore-tti-inline-compatible`.
> * LLVM doesn't want to inline functions combined with
>   `-fno-delete-null-pointer-checks` with code compiled without. So we
>   remove this command when compiling helpers. I think this should be
>   okay since this is one of the hardening features and we shouldn't have
>   null pointer dereferences in these helpers.
>
> The checks can also be bypassed with force inlining (`__always_inline`)
> but the behaviour is the same with extra options.
>
> Co-developed-by: Boqun Feng <boqun.feng@gmail.com>
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> Signed-off-by: Gary Guo <gary@garyguo.net>


Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
Tested-by: Andreas Hindborg <a.hindborg@kernel.org>

Tested with downstream rnull tree, works as expected.

Best regards,
Andreas Hindborg

Re: [PATCH v2 2/3] kbuild: rust: provide an option to inline C helpers into Rust

[Tamir Duberstein]

On Sun, Jan 5, 2025 at 3:03 PM Gary Guo <gary@garyguo.net> wrote:
>
> A new Kconfig option, `RUST_INLINE_HELPERS` is added to allow C helpers
> (which was created to allow Rust to call into inline/macro C functions

s/was/were/

> without having to re-implement the logic in Rust) to be inlined into
> Rust crates without performing a global LTO.

superfluous article (drop the "a")

>
> If the option is enabled, the following is performed:
> * For helpers, instead of compiling them to object file to be linked
>   into vmlinux, we compile them to LLVM IR.
> * The LLVM IR is patched to add `linkonce_odr` linkage. This linkage
>   means that the function is inlineable (effect of `_odr`), and the
>   symbols generated will have weak linkage if emitted into object file
>   (important since as later described, we might have multiple copies of
>   the same symbol) and it will may be discarded if it is not invoked or
>   all invocations are inlined.
> * The LLVM IR is compiled to bitcode (This is step is not necessary, but
>   is a performance optimisation to prevent LLVM from always have to
>   reparse the same IR).
> * When a Rust crate is compiled, instead of generating object file, we
>   ask LLVM bitcode to be generated.
> * llvm-link is invoked to combine the helper bitcode with the crate
>   bitcode. This step is similar to LTO, but this is much faster since it
>   only needs to inline the helpers.
> * clang is invoked to turn the combined bitcode into object file.

There's a mix of active ("we compile") and passive ("is patched")
voices here, and some missing articles or unclear use of singular
where plural is more appropriate (e.g. object file should be object
files).

>
> Some caveats with the option:
> * clang and Rust doesn't have the exact target string. Manual inspection
>   shows that they should be compatible, but since they are not exactly
>   the same LLVM seems to prefer not inlining them. This is bypassed with
>   `--ignore-tti-inline-compatible`.

Do we know why the target strings aren't the same? Are there citations
that could be included here?

> * LLVM doesn't want to inline functions combined with

s/combined/compiled/

>   `-fno-delete-null-pointer-checks` with code compiled without. So we
>   remove this command when compiling helpers. I think this should be

s/command/flag/

>   okay since this is one of the hardening features and we shouldn't have
>   null pointer dereferences in these helpers.

Is the implication that kernel C is compiled with this flag, but Rust
code isn't? Do we know why?

> The checks can also be bypassed with force inlining (`__always_inline`)
> but the behaviour is the same with extra options.

If the behavior is the same, wouldn't it be better to use
`__always_inline`? Otherwise LLVM's behavior might change such that
inlining is lost and we won't notice.

>
> Co-developed-by: Boqun Feng <boqun.feng@gmail.com>
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> Signed-off-by: Gary Guo <gary@garyguo.net>
> ---
>  Makefile                     |  4 +++-
>  lib/Kconfig.debug            | 14 ++++++++++++++
>  rust/Makefile                | 31 +++++++++++++++++++++++++++----
>  rust/exports.c               |  5 ++++-
>  rust/helpers/blk.c           |  5 +++--
>  rust/helpers/bug.c           |  3 ++-
>  rust/helpers/build_bug.c     |  3 ++-
>  rust/helpers/cred.c          |  5 +++--
>  rust/helpers/err.c           |  7 ++++---
>  rust/helpers/fs.c            |  3 ++-
>  rust/helpers/helpers.h       | 12 ++++++++++++
>  rust/helpers/jump_label.c    |  3 ++-
>  rust/helpers/kunit.c         |  3 ++-
>  rust/helpers/mutex.c         |  7 ++++---
>  rust/helpers/page.c          |  8 +++++---
>  rust/helpers/pid_namespace.c |  7 ++++---
>  rust/helpers/rbtree.c        |  6 ++++--
>  rust/helpers/refcount.c      |  7 ++++---
>  rust/helpers/security.c      |  7 ++++---
>  rust/helpers/signal.c        |  3 ++-
>  rust/helpers/slab.c          |  5 +++--
>  rust/helpers/spinlock.c      | 11 ++++++-----
>  rust/helpers/task.c          | 23 ++++++++++++-----------
>  rust/helpers/uaccess.c       |  9 +++++----
>  rust/helpers/vmalloc.c       |  3 ++-
>  rust/helpers/wait.c          |  3 ++-
>  rust/helpers/workqueue.c     |  9 ++++++---
>  scripts/Makefile.build       |  5 ++++-
>  28 files changed, 147 insertions(+), 64 deletions(-)
>  create mode 100644 rust/helpers/helpers.h
>
> diff --git a/Makefile b/Makefile
> index e5b8a8832c0c..5fbb5e829f81 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -509,6 +509,8 @@ OBJCOPY             = $(LLVM_PREFIX)llvm-objcopy$(LLVM_SUFFIX)
>  OBJDUMP                = $(LLVM_PREFIX)llvm-objdump$(LLVM_SUFFIX)
>  READELF                = $(LLVM_PREFIX)llvm-readelf$(LLVM_SUFFIX)
>  STRIP          = $(LLVM_PREFIX)llvm-strip$(LLVM_SUFFIX)
> +LLVM_LINK      = $(LLVM_PREFIX)llvm-link$(LLVM_SUFFIX)
> +LLVM_AS        = $(LLVM_PREFIX)llvm-as$(LLVM_SUFFIX)
>  else
>  CC             = $(CROSS_COMPILE)gcc
>  LD             = $(CROSS_COMPILE)ld
> @@ -616,7 +618,7 @@ export RUSTC_BOOTSTRAP := 1
>  export CLIPPY_CONF_DIR := $(srctree)
>
>  export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE LD CC HOSTPKG_CONFIG
> -export RUSTC RUSTDOC RUSTFMT RUSTC_OR_CLIPPY_QUIET RUSTC_OR_CLIPPY BINDGEN
> +export RUSTC RUSTDOC RUSTFMT RUSTC_OR_CLIPPY_QUIET RUSTC_OR_CLIPPY BINDGEN LLVM_LINK LLVM_AS
>  export HOSTRUSTC KBUILD_HOSTRUSTFLAGS
>  export CPP AR NM STRIP OBJCOPY OBJDUMP READELF PAHOLE RESOLVE_BTFIDS LEX YACC AWK INSTALLKERNEL
>  export PERL PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index f3d723705879..8a8834128f13 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -3253,6 +3253,20 @@ config RUST_KERNEL_DOCTESTS
>
>           If unsure, say N.
>
> +config RUST_INLINE_HELPERS
> +    bool "Inline C helpers into Rust crates"
> +    depends on RUST && RUSTC_CLANG_LLVM_COMPATIBLE
> +    help
> +        Links C helpers into with Rust crates through LLVM IR.
> +
> +        If this option is enabled, instead of generating object files directly,
> +        rustc is asked to produce LLVM IR, which is then linked together with
> +        the LLVM IR of C helpers, before object file is generated.

s/IR/bitcode/g

Right?

> +
> +        This requires a matching LLVM version for Clang and rustc.
> +
> +        If unsure, say N.
> +
>  endmenu # "Rust"
>
>  endmenu # Kernel hacking
> diff --git a/rust/Makefile b/rust/Makefile
> index a40a3936126d..8a3d61e5a799 100644
> --- a/rust/Makefile
> +++ b/rust/Makefile
> @@ -6,9 +6,14 @@ rustdoc_output := $(objtree)/Documentation/output/rust/rustdoc
>  obj-$(CONFIG_RUST) += core.o compiler_builtins.o ffi.o
>  always-$(CONFIG_RUST) += exports_core_generated.h
>
> +ifdef CONFIG_RUST_INLINE_HELPERS
> +always-$(CONFIG_RUST) += helpers/helpers.bc
> +else
> +obj-$(CONFIG_RUST) += helpers/helpers.o
> +always-$(CONFIG_RUST) += exports_helpers_generated.h
> +endif
>  # Missing prototypes are expected in the helpers since these are exported
>  # for Rust only, thus there is no header nor prototypes.
> -obj-$(CONFIG_RUST) += helpers/helpers.o
>  CFLAGS_REMOVE_helpers/helpers.o = -Wmissing-prototypes -Wmissing-declarations

Should this also move up into the else branch above?

>  always-$(CONFIG_RUST) += libmacros.so
> @@ -16,8 +21,7 @@ no-clean-files += libmacros.so
>
>  always-$(CONFIG_RUST) += bindings/bindings_generated.rs bindings/bindings_helpers_generated.rs
>  obj-$(CONFIG_RUST) += bindings.o kernel.o
> -always-$(CONFIG_RUST) += exports_helpers_generated.h \
> -    exports_bindings_generated.h exports_kernel_generated.h
> +always-$(CONFIG_RUST) += exports_bindings_generated.h exports_kernel_generated.h
>
>  always-$(CONFIG_RUST) += uapi/uapi_generated.rs
>  obj-$(CONFIG_RUST) += uapi.o
> @@ -329,6 +333,18 @@ $(obj)/bindings/bindings_helpers_generated.rs: private bindgen_target_extra = ;
>  $(obj)/bindings/bindings_helpers_generated.rs: $(src)/helpers/helpers.c FORCE
>         $(call if_changed_dep,bindgen)
>
> +# When compiling helpers, filter `-fno-delete-null-pointer-checks` since LLVM
> +# prevents inlining such functions to be inlined into functions compiled
> +# without the option (e.g. Rust functions).

Duplicate mention of inlining.

> +quiet_cmd_rust_helper = HELPER  $@
> +      cmd_rust_helper = \
> +       $(CC) $(filter-out -fno-delete-null-pointer-checks $(CFLAGS_REMOVE_helpers/helpers.o), $(c_flags)) -S $< -emit-llvm -o $(patsubst %.bc,%.ll,$@); \
> +       sed -i 's/^define dso_local/define linkonce_odr dso_local/g' $(patsubst %.bc,%.ll,$@); \

This isn't mentioned in the comment.

> +       $(LLVM_AS) $(patsubst %.bc,%.ll,$@) -o $@
> +
> +$(obj)/helpers/helpers.bc: $(obj)/helpers/helpers.c FORCE
> +       +$(call if_changed_dep,rust_helper)

Should all these rules be defined iff CONFIG_RUST_INLINE_HELPERS?
Always defining them seems like it could lead to subtle bugs, but
perhaps there's Makefile precedent I'm not aware of.

> +
>  quiet_cmd_exports = EXPORTS $@
>        cmd_exports = \
>         $(NM) -p --defined-only $< \
> @@ -373,11 +389,13 @@ quiet_cmd_rustc_library = $(if $(skip_clippy),RUSTC,$(RUSTC_OR_CLIPPY_QUIET)) L
>         OBJTREE=$(abspath $(objtree)) \
>         $(if $(skip_clippy),$(RUSTC),$(RUSTC_OR_CLIPPY)) \
>                 $(filter-out $(skip_flags),$(rust_flags) $(rustc_target_flags)) \
> -               --emit=dep-info=$(depfile) --emit=obj=$@ \
> +               --emit=dep-info=$(depfile) --emit=$(if $(link_helper),llvm-bc=$(patsubst %.o,%.bc,$@),obj=$@) \
>                 --emit=metadata=$(dir $@)$(patsubst %.o,lib%.rmeta,$(notdir $@)) \
>                 --crate-type rlib -L$(objtree)/$(obj) \
>                 --crate-name $(patsubst %.o,%,$(notdir $@)) $< \
>                 --sysroot=/dev/null \
> +       $(if $(link_helper),;$(LLVM_LINK) $(patsubst %.o,%.bc,$@) $(obj)/helpers/helpers.bc -o $(patsubst %.o,%.m.bc,$@); \
> +               $(CC) $(CLANG_FLAGS) $(KBUILD_CFLAGS) -mllvm=--ignore-tti-inline-compatible -c $(patsubst %.o,%.m.bc,$@) -o $@) \
>         $(if $(rustc_objcopy),;$(OBJCOPY) $(rustc_objcopy) $@) \
>         $(cmd_objtool)
>
> @@ -450,4 +468,9 @@ ifdef CONFIG_JUMP_LABEL
>  $(obj)/kernel.o: $(obj)/kernel/generated_arch_static_branch_asm.rs
>  endif
>
> +ifdef CONFIG_RUST_INLINE_HELPERS
> +$(obj)/kernel.o: private link_helper = 1
> +$(obj)/kernel.o: $(obj)/helpers/helpers.bc
> +endif

Can this be combined with the other `ifdef CONFIG_RUST_INLINE_HELPERS`?

> +
>  endif # CONFIG_RUST
> diff --git a/rust/exports.c b/rust/exports.c
> index 587f0e776aba..1b52460b0f4e 100644
> --- a/rust/exports.c
> +++ b/rust/exports.c
> @@ -16,10 +16,13 @@
>  #define EXPORT_SYMBOL_RUST_GPL(sym) extern int sym; EXPORT_SYMBOL_GPL(sym)
>
>  #include "exports_core_generated.h"
> -#include "exports_helpers_generated.h"
>  #include "exports_bindings_generated.h"
>  #include "exports_kernel_generated.h"
>
> +#ifndef CONFIG_RUST_INLINE_HELPERS
> +#include "exports_helpers_generated.h"
> +#endif
> +
>  // For modules using `rust/build_error.rs`.
>  #ifdef CONFIG_RUST_BUILD_ASSERT_ALLOW
>  EXPORT_SYMBOL_RUST_GPL(rust_build_error);
> diff --git a/rust/helpers/blk.c b/rust/helpers/blk.c
> index cc9f4e6a2d23..a96e5ddf384d 100644
> --- a/rust/helpers/blk.c
> +++ b/rust/helpers/blk.c
> @@ -2,13 +2,14 @@
>
>  #include <linux/blk-mq.h>
>  #include <linux/blkdev.h>
> +#include "helpers.h"
>
> -void *rust_helper_blk_mq_rq_to_pdu(struct request *rq)
> +__rust_helper void *rust_helper_blk_mq_rq_to_pdu(struct request *rq)
>  {
>         return blk_mq_rq_to_pdu(rq);
>  }
>
> -struct request *rust_helper_blk_mq_rq_from_pdu(void *pdu)
> +__rust_helper struct request *rust_helper_blk_mq_rq_from_pdu(void *pdu)
>  {
>         return blk_mq_rq_from_pdu(pdu);
>  }
> diff --git a/rust/helpers/bug.c b/rust/helpers/bug.c
> index e2d13babc737..3bff5b730ca8 100644
> --- a/rust/helpers/bug.c
> +++ b/rust/helpers/bug.c
> @@ -1,8 +1,9 @@
>  // SPDX-License-Identifier: GPL-2.0
>
>  #include <linux/bug.h>
> +#include "helpers.h"
>
> -__noreturn void rust_helper_BUG(void)
> +__rust_helper __noreturn void rust_helper_BUG(void)
>  {
>         BUG();
>  }
> diff --git a/rust/helpers/build_bug.c b/rust/helpers/build_bug.c
> index 44e579488037..9dc273fd8db1 100644
> --- a/rust/helpers/build_bug.c
> +++ b/rust/helpers/build_bug.c
> @@ -1,8 +1,9 @@
>  // SPDX-License-Identifier: GPL-2.0
>
>  #include <linux/errname.h>
> +#include "helpers.h"
>
> -const char *rust_helper_errname(int err)
> +__rust_helper const char *rust_helper_errname(int err)
>  {
>         return errname(err);
>  }
> diff --git a/rust/helpers/cred.c b/rust/helpers/cred.c
> index fde7ae20cdd1..9dceeb7f8c87 100644
> --- a/rust/helpers/cred.c
> +++ b/rust/helpers/cred.c
> @@ -1,13 +1,14 @@
>  // SPDX-License-Identifier: GPL-2.0
>
>  #include <linux/cred.h>
> +#include "helpers.h"
>
> -const struct cred *rust_helper_get_cred(const struct cred *cred)
> +__rust_helper const struct cred *rust_helper_get_cred(const struct cred *cred)
>  {
>         return get_cred(cred);
>  }
>
> -void rust_helper_put_cred(const struct cred *cred)
> +__rust_helper void rust_helper_put_cred(const struct cred *cred)
>  {
>         put_cred(cred);
>  }
> diff --git a/rust/helpers/err.c b/rust/helpers/err.c
> index 544c7cb86632..b05516fce504 100644
> --- a/rust/helpers/err.c
> +++ b/rust/helpers/err.c
> @@ -1,18 +1,19 @@
>  // SPDX-License-Identifier: GPL-2.0
>
>  #include <linux/err.h>
> +#include "helpers.h"
>
> -__force void *rust_helper_ERR_PTR(long err)
> +__rust_helper __force void *rust_helper_ERR_PTR(long err)
>  {
>         return ERR_PTR(err);
>  }
>
> -bool rust_helper_IS_ERR(__force const void *ptr)
> +__rust_helper bool rust_helper_IS_ERR(__force const void *ptr)
>  {
>         return IS_ERR(ptr);
>  }
>
> -long rust_helper_PTR_ERR(__force const void *ptr)
> +__rust_helper long rust_helper_PTR_ERR(__force const void *ptr)
>  {
>         return PTR_ERR(ptr);
>  }
> diff --git a/rust/helpers/fs.c b/rust/helpers/fs.c
> index a75c96763372..7d44bda94203 100644
> --- a/rust/helpers/fs.c
> +++ b/rust/helpers/fs.c
> @@ -5,8 +5,9 @@
>   */
>
>  #include <linux/fs.h>
> +#include "helpers.h"
>
> -struct file *rust_helper_get_file(struct file *f)
> +__rust_helper struct file *rust_helper_get_file(struct file *f)
>  {
>         return get_file(f);
>  }
> diff --git a/rust/helpers/helpers.h b/rust/helpers/helpers.h
> new file mode 100644
> index 000000000000..9248692146f3
> --- /dev/null
> +++ b/rust/helpers/helpers.h
> @@ -0,0 +1,12 @@
> +#ifndef RUST_HELPERS_H
> +#define RUST_HELPERS_H
> +
> +#include <linux/compiler_types.h>
> +
> +#ifdef __BINDGEN__
> +#define __rust_helper
> +#else
> +#define __rust_helper inline
> +#endif

Could you mention this in the commit message? It's not obvious to me
what this does and why it depends on __BINDGEN__ rather than
CONFIG_RUST_INLINE_HELPERS.


> +#endif
> diff --git a/rust/helpers/jump_label.c b/rust/helpers/jump_label.c
> index fc1f1e0df08e..c97a66c51cce 100644
> --- a/rust/helpers/jump_label.c
> +++ b/rust/helpers/jump_label.c
> @@ -5,9 +5,10 @@
>   */
>
>  #include <linux/jump_label.h>
> +#include "helpers.h"
>
>  #ifndef CONFIG_JUMP_LABEL
> -int rust_helper_static_key_count(struct static_key *key)
> +__rust_helper int rust_helper_static_key_count(struct static_key *key)
>  {
>         return static_key_count(key);
>  }
> diff --git a/rust/helpers/kunit.c b/rust/helpers/kunit.c
> index b85a4d394c11..f3621a564626 100644
> --- a/rust/helpers/kunit.c
> +++ b/rust/helpers/kunit.c
> @@ -1,8 +1,9 @@
>  // SPDX-License-Identifier: GPL-2.0
>
>  #include <kunit/test-bug.h>
> +#include "helpers.h"
>
> -struct kunit *rust_helper_kunit_get_current_test(void)
> +__rust_helper struct kunit *rust_helper_kunit_get_current_test(void)
>  {
>         return kunit_get_current_test();
>  }
> diff --git a/rust/helpers/mutex.c b/rust/helpers/mutex.c
> index 7e00680958ef..6259b5c959f6 100644
> --- a/rust/helpers/mutex.c
> +++ b/rust/helpers/mutex.c
> @@ -1,14 +1,15 @@
>  // SPDX-License-Identifier: GPL-2.0
>
>  #include <linux/mutex.h>
> +#include "helpers.h"
>
> -void rust_helper_mutex_lock(struct mutex *lock)
> +__rust_helper void rust_helper_mutex_lock(struct mutex *lock)
>  {
>         mutex_lock(lock);
>  }
>
> -void rust_helper___mutex_init(struct mutex *mutex, const char *name,
> -                             struct lock_class_key *key)
> +__rust_helper void rust_helper___mutex_init(struct mutex *mutex, const char *name,
> +                                           struct lock_class_key *key)
>  {
>         __mutex_init(mutex, name, key);
>  }
> diff --git a/rust/helpers/page.c b/rust/helpers/page.c
> index b3f2b8fbf87f..cf7deea25cfa 100644
> --- a/rust/helpers/page.c
> +++ b/rust/helpers/page.c
> @@ -2,18 +2,20 @@
>
>  #include <linux/gfp.h>
>  #include <linux/highmem.h>
> +#include "helpers.h"
>
> -struct page *rust_helper_alloc_pages(gfp_t gfp_mask, unsigned int order)
> +__rust_helper struct page *rust_helper_alloc_pages(gfp_t gfp_mask,
> +                                                  unsigned int order)
>  {
>         return alloc_pages(gfp_mask, order);
>  }
>
> -void *rust_helper_kmap_local_page(struct page *page)
> +__rust_helper void *rust_helper_kmap_local_page(struct page *page)
>  {
>         return kmap_local_page(page);
>  }
>
> -void rust_helper_kunmap_local(const void *addr)
> +__rust_helper void rust_helper_kunmap_local(const void *addr)
>  {
>         kunmap_local(addr);
>  }
> diff --git a/rust/helpers/pid_namespace.c b/rust/helpers/pid_namespace.c
> index f41482bdec9a..f066fa4e7872 100644
> --- a/rust/helpers/pid_namespace.c
> +++ b/rust/helpers/pid_namespace.c
> @@ -2,19 +2,20 @@
>
>  #include <linux/pid_namespace.h>
>  #include <linux/cleanup.h>
> +#include "helpers.h"
>
> -struct pid_namespace *rust_helper_get_pid_ns(struct pid_namespace *ns)
> +__rust_helper struct pid_namespace *rust_helper_get_pid_ns(struct pid_namespace *ns)
>  {
>         return get_pid_ns(ns);
>  }
>
> -void rust_helper_put_pid_ns(struct pid_namespace *ns)
> +__rust_helper void rust_helper_put_pid_ns(struct pid_namespace *ns)
>  {
>         put_pid_ns(ns);
>  }
>
>  /* Get a reference on a task's pid namespace. */
> -struct pid_namespace *rust_helper_task_get_pid_ns(struct task_struct *task)
> +__rust_helper struct pid_namespace *rust_helper_task_get_pid_ns(struct task_struct *task)
>  {
>         struct pid_namespace *pid_ns;
>
> diff --git a/rust/helpers/rbtree.c b/rust/helpers/rbtree.c
> index 6d404b84a9b5..784d8796aa1c 100644
> --- a/rust/helpers/rbtree.c
> +++ b/rust/helpers/rbtree.c
> @@ -1,9 +1,11 @@
>  // SPDX-License-Identifier: GPL-2.0
>
>  #include <linux/rbtree.h>
> +#include "helpers.h"
>
> -void rust_helper_rb_link_node(struct rb_node *node, struct rb_node *parent,
> -                             struct rb_node **rb_link)
> +__rust_helper void rust_helper_rb_link_node(struct rb_node *node,
> +                                           struct rb_node *parent,
> +                                           struct rb_node **rb_link)
>  {
>         rb_link_node(node, parent, rb_link);
>  }
> diff --git a/rust/helpers/refcount.c b/rust/helpers/refcount.c
> index d6adbd2e45a1..ad80e153bf37 100644
> --- a/rust/helpers/refcount.c
> +++ b/rust/helpers/refcount.c
> @@ -1,18 +1,19 @@
>  // SPDX-License-Identifier: GPL-2.0
>
>  #include <linux/refcount.h>
> +#include "helpers.h"
>
> -refcount_t rust_helper_REFCOUNT_INIT(int n)
> +__rust_helper refcount_t rust_helper_REFCOUNT_INIT(int n)
>  {
>         return (refcount_t)REFCOUNT_INIT(n);
>  }
>
> -void rust_helper_refcount_inc(refcount_t *r)
> +__rust_helper void rust_helper_refcount_inc(refcount_t *r)
>  {
>         refcount_inc(r);
>  }
>
> -bool rust_helper_refcount_dec_and_test(refcount_t *r)
> +__rust_helper bool rust_helper_refcount_dec_and_test(refcount_t *r)
>  {
>         return refcount_dec_and_test(r);
>  }
> diff --git a/rust/helpers/security.c b/rust/helpers/security.c
> index 239e5b4745fe..b0c14e1b3aff 100644
> --- a/rust/helpers/security.c
> +++ b/rust/helpers/security.c
> @@ -1,19 +1,20 @@
>  // SPDX-License-Identifier: GPL-2.0
>
>  #include <linux/security.h>
> +#include "helpers.h"
>
>  #ifndef CONFIG_SECURITY
> -void rust_helper_security_cred_getsecid(const struct cred *c, u32 *secid)
> +__rust_helper void rust_helper_security_cred_getsecid(const struct cred *c, u32 *secid)
>  {
>         security_cred_getsecid(c, secid);
>  }
>
> -int rust_helper_security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
> +__rust_helper int rust_helper_security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
>  {
>         return security_secid_to_secctx(secid, secdata, seclen);
>  }
>
> -void rust_helper_security_release_secctx(char *secdata, u32 seclen)
> +__rust_helper void rust_helper_security_release_secctx(char *secdata, u32 seclen)
>  {
>         security_release_secctx(secdata, seclen);
>  }
> diff --git a/rust/helpers/signal.c b/rust/helpers/signal.c
> index 1a6bbe9438e2..67d3fe8d5132 100644
> --- a/rust/helpers/signal.c
> +++ b/rust/helpers/signal.c
> @@ -1,8 +1,9 @@
>  // SPDX-License-Identifier: GPL-2.0
>
>  #include <linux/sched/signal.h>
> +#include "helpers.h"
>
> -int rust_helper_signal_pending(struct task_struct *t)
> +__rust_helper int rust_helper_signal_pending(struct task_struct *t)
>  {
>         return signal_pending(t);
>  }
> diff --git a/rust/helpers/slab.c b/rust/helpers/slab.c
> index a842bfbddcba..f8776884b729 100644
> --- a/rust/helpers/slab.c
> +++ b/rust/helpers/slab.c
> @@ -1,14 +1,15 @@
>  // SPDX-License-Identifier: GPL-2.0
>
>  #include <linux/slab.h>
> +#include "helpers.h"
>
> -void * __must_check __realloc_size(2)
> +__rust_helper void *__must_check __realloc_size(2)
>  rust_helper_krealloc(const void *objp, size_t new_size, gfp_t flags)
>  {
>         return krealloc(objp, new_size, flags);
>  }
>
> -void * __must_check __realloc_size(2)
> +__rust_helper void * __must_check __realloc_size(2)
>  rust_helper_kvrealloc(const void *p, size_t size, gfp_t flags)
>  {
>         return kvrealloc(p, size, flags);
> diff --git a/rust/helpers/spinlock.c b/rust/helpers/spinlock.c
> index 5971fdf6f755..567161c19a16 100644
> --- a/rust/helpers/spinlock.c
> +++ b/rust/helpers/spinlock.c
> @@ -1,9 +1,10 @@
>  // SPDX-License-Identifier: GPL-2.0
>
>  #include <linux/spinlock.h>
> +#include "helpers.h"
>
> -void rust_helper___spin_lock_init(spinlock_t *lock, const char *name,
> -                                 struct lock_class_key *key)
> +__rust_helper void rust_helper___spin_lock_init(spinlock_t *lock, const char *name,
> +                                               struct lock_class_key *key)
>  {
>  #ifdef CONFIG_DEBUG_SPINLOCK
>  # if defined(CONFIG_PREEMPT_RT)
> @@ -16,17 +17,17 @@ void rust_helper___spin_lock_init(spinlock_t *lock, const char *name,
>  #endif /* CONFIG_DEBUG_SPINLOCK */
>  }
>
> -void rust_helper_spin_lock(spinlock_t *lock)
> +__rust_helper void rust_helper_spin_lock(spinlock_t *lock)
>  {
>         spin_lock(lock);
>  }
>
> -void rust_helper_spin_unlock(spinlock_t *lock)
> +__rust_helper void rust_helper_spin_unlock(spinlock_t *lock)
>  {
>         spin_unlock(lock);
>  }
>
> -int rust_helper_spin_trylock(spinlock_t *lock)
> +__rust_helper int rust_helper_spin_trylock(spinlock_t *lock)
>  {
>         return spin_trylock(lock);
>  }
> diff --git a/rust/helpers/task.c b/rust/helpers/task.c
> index 31c33ea2dce6..741e1a4b15ee 100644
> --- a/rust/helpers/task.c
> +++ b/rust/helpers/task.c
> @@ -1,56 +1,57 @@
>  // SPDX-License-Identifier: GPL-2.0
>
>  #include <linux/sched/task.h>
> +#include "helpers.h"
>
> -struct task_struct *rust_helper_get_current(void)
> +__rust_helper struct task_struct *rust_helper_get_current(void)
>  {
>         return current;
>  }
>
> -void rust_helper_get_task_struct(struct task_struct *t)
> +__rust_helper void rust_helper_get_task_struct(struct task_struct *t)
>  {
>         get_task_struct(t);
>  }
>
> -void rust_helper_put_task_struct(struct task_struct *t)
> +__rust_helper void rust_helper_put_task_struct(struct task_struct *t)
>  {
>         put_task_struct(t);
>  }
>
> -kuid_t rust_helper_task_uid(struct task_struct *task)
> +__rust_helper kuid_t rust_helper_task_uid(struct task_struct *task)
>  {
>         return task_uid(task);
>  }
>
> -kuid_t rust_helper_task_euid(struct task_struct *task)
> +__rust_helper kuid_t rust_helper_task_euid(struct task_struct *task)
>  {
>         return task_euid(task);
>  }
>
>  #ifndef CONFIG_USER_NS
> -uid_t rust_helper_from_kuid(struct user_namespace *to, kuid_t uid)
> +__rust_helper uid_t rust_helper_from_kuid(struct user_namespace *to, kuid_t uid)
>  {
>         return from_kuid(to, uid);
>  }
>  #endif /* CONFIG_USER_NS */
>
> -bool rust_helper_uid_eq(kuid_t left, kuid_t right)
> +__rust_helper bool rust_helper_uid_eq(kuid_t left, kuid_t right)
>  {
>         return uid_eq(left, right);
>  }
>
> -kuid_t rust_helper_current_euid(void)
> +__rust_helper kuid_t rust_helper_current_euid(void)
>  {
>         return current_euid();
>  }
>
> -struct user_namespace *rust_helper_current_user_ns(void)
> +__rust_helper struct user_namespace *rust_helper_current_user_ns(void)
>  {
>         return current_user_ns();
>  }
>
> -pid_t rust_helper_task_tgid_nr_ns(struct task_struct *tsk,
> -                                 struct pid_namespace *ns)
> +__rust_helper pid_t rust_helper_task_tgid_nr_ns(struct task_struct *tsk,
> +                                               struct pid_namespace *ns)
>  {
>         return task_tgid_nr_ns(tsk, ns);
>  }
> diff --git a/rust/helpers/uaccess.c b/rust/helpers/uaccess.c
> index f49076f813cd..08bf1851c06a 100644
> --- a/rust/helpers/uaccess.c
> +++ b/rust/helpers/uaccess.c
> @@ -1,15 +1,16 @@
>  // SPDX-License-Identifier: GPL-2.0
>
>  #include <linux/uaccess.h>
> +#include "helpers.h"
>
> -unsigned long rust_helper_copy_from_user(void *to, const void __user *from,
> -                                        unsigned long n)
> +__rust_helper unsigned long
> +rust_helper_copy_from_user(void *to, const void __user *from, unsigned long n)
>  {
>         return copy_from_user(to, from, n);
>  }
>
> -unsigned long rust_helper_copy_to_user(void __user *to, const void *from,
> -                                      unsigned long n)
> +__rust_helper unsigned long
> +rust_helper_copy_to_user(void __user *to, const void *from, unsigned long n)
>  {
>         return copy_to_user(to, from, n);
>  }
> diff --git a/rust/helpers/vmalloc.c b/rust/helpers/vmalloc.c
> index 80d34501bbc0..6d258baa4c9a 100644
> --- a/rust/helpers/vmalloc.c
> +++ b/rust/helpers/vmalloc.c
> @@ -1,8 +1,9 @@
>  // SPDX-License-Identifier: GPL-2.0
>
>  #include <linux/vmalloc.h>
> +#include "helpers.h"
>
> -void * __must_check __realloc_size(2)
> +__rust_helper void * __must_check __realloc_size(2)
>  rust_helper_vrealloc(const void *p, size_t size, gfp_t flags)
>  {
>         return vrealloc(p, size, flags);
> diff --git a/rust/helpers/wait.c b/rust/helpers/wait.c
> index ae48e33d9da3..7beaa85494bc 100644
> --- a/rust/helpers/wait.c
> +++ b/rust/helpers/wait.c
> @@ -1,8 +1,9 @@
>  // SPDX-License-Identifier: GPL-2.0
>
>  #include <linux/wait.h>
> +#include "helpers.h"
>
> -void rust_helper_init_wait(struct wait_queue_entry *wq_entry)
> +__rust_helper void rust_helper_init_wait(struct wait_queue_entry *wq_entry)
>  {
>         init_wait(wq_entry);
>  }
> diff --git a/rust/helpers/workqueue.c b/rust/helpers/workqueue.c
> index b2b82753509b..4256f8c91ce6 100644
> --- a/rust/helpers/workqueue.c
> +++ b/rust/helpers/workqueue.c
> @@ -1,10 +1,13 @@
>  // SPDX-License-Identifier: GPL-2.0
>
>  #include <linux/workqueue.h>
> +#include "helpers.h"
>
> -void rust_helper_init_work_with_key(struct work_struct *work, work_func_t func,
> -                                   bool onstack, const char *name,
> -                                   struct lock_class_key *key)
> +__rust_helper void rust_helper_init_work_with_key(struct work_struct *work,
> +                                                 work_func_t func,
> +                                                 bool onstack,
> +                                                 const char *name,
> +                                                 struct lock_class_key *key)
>  {
>         __init_work(work, onstack);
>         work->data = (atomic_long_t)WORK_DATA_INIT();
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index c16e4cf54d77..77c204449d1b 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -239,7 +239,10 @@ rust_common_cmd = \
>  # would not match each other.
>
>  quiet_cmd_rustc_o_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
> -      cmd_rustc_o_rs = $(rust_common_cmd) --emit=obj=$@ $< $(cmd_objtool)
> +      cmd_rustc_o_rs = $(rust_common_cmd) --emit=$(if $(CONFIG_RUST_INLINE_HELPERS),llvm-bc=$(patsubst %.o,%.bc,$@),obj=$@) $< \
> +       $(if $(CONFIG_RUST_INLINE_HELPERS),;$(LLVM_LINK) $(patsubst %.o,%.bc,$@) $(objtree)/rust/helpers/helpers.bc -o $(patsubst %.o,%.m.bc,$@); \
> +               $(CC) $(CLANG_FLAGS) $(KBUILD_CFLAGS) -mllvm=--ignore-tti-inline-compatible -c $(patsubst %.o,%.m.bc,$@) -o $@) \
> +       $(cmd_objtool)
>
>  define rule_rustc_o_rs
>         $(call cmd_and_fixdep,rustc_o_rs)
> --
> 2.47.0
>

Very cool!
Tamir

Re: [PATCH v2 2/3] kbuild: rust: provide an option to inline C helpers into Rust

[Gary Guo]

On Thu, 6 Mar 2025 18:00:10 -0500
Tamir Duberstein <tamird@gmail.com> wrote:

> > Some caveats with the option:
> > * clang and Rust doesn't have the exact target string. Manual inspection
> >   shows that they should be compatible, but since they are not exactly
> >   the same LLVM seems to prefer not inlining them. This is bypassed with
> >   `--ignore-tti-inline-compatible`.  
> 
> Do we know why the target strings aren't the same? Are there citations
> that could be included here?

I've added an explaination in new patch series.

> 
> >   okay since this is one of the hardening features and we shouldn't have
> >   null pointer dereferences in these helpers.  
> 
> Is the implication that kernel C is compiled with this flag, but Rust
> code isn't? Do we know why?

ABI is compatible with and without this. I've added a short
explaination in the new version.

> > The checks can also be bypassed with force inlining (`__always_inline`)
> > but the behaviour is the same with extra options.  
> 
> If the behavior is the same, wouldn't it be better to use
> `__always_inline`? Otherwise LLVM's behavior might change such that
> inlining is lost and we won't notice.

If everything works as expected, then the behaviour is the same, but
not focing inline can be used to detect mistakes, e.g. when an inline
function gets too large.

Most C side don't use `__always_inline` but rather just `inline` so I
want to keep helpers the same.

> >
> > +config RUST_INLINE_HELPERS
> > +    bool "Inline C helpers into Rust crates"
> > +    depends on RUST && RUSTC_CLANG_LLVM_COMPATIBLE
> > +    help
> > +        Links C helpers into with Rust crates through LLVM IR.
> > +
> > +        If this option is enabled, instead of generating object files directly,
> > +        rustc is asked to produce LLVM IR, which is then linked together with
> > +        the LLVM IR of C helpers, before object file is generated.  
> 
> s/IR/bitcode/g
> 
> Right?

I'd rather keep "IR" here as it's a more general concept.

Bitcode generation is an implementation detail really and user
shouldn't care. If we remove bitcode steps then the whole idea still
works as expected.

> >  # Missing prototypes are expected in the helpers since these are exported
> >  # for Rust only, thus there is no header nor prototypes.
> > -obj-$(CONFIG_RUST) += helpers/helpers.o
> >  CFLAGS_REMOVE_helpers/helpers.o = -Wmissing-prototypes -Wmissing-declarations  
> 
> Should this also move up into the else branch above?
> 
> > +       $(LLVM_AS) $(patsubst %.bc,%.ll,$@) -o $@
> > +
> > +$(obj)/helpers/helpers.bc: $(obj)/helpers/helpers.c FORCE
> > +       +$(call if_changed_dep,rust_helper)  
> 
> Should all these rules be defined iff CONFIG_RUST_INLINE_HELPERS?
> Always defining them seems like it could lead to subtle bugs, but
> perhaps there's Makefile precedent I'm not aware of.

I don't think that's needed the way Kbuild works. For all C source
files, we have targets for all .o files regardless if a config is
enabled (enabling a config merely adds the corresponding .o to obj-y).
So I don't think this is needed for helpers either.

> > +ifdef CONFIG_RUST_INLINE_HELPERS
> > +$(obj)/kernel.o: private link_helper = 1
> > +$(obj)/kernel.o: $(obj)/helpers/helpers.bc
> > +endif  
> 
> Can this be combined with the other `ifdef CONFIG_RUST_INLINE_HELPERS`?

I want all kernel.o related lines to be closer together.

> > +#ifndef RUST_HELPERS_H
> > +#define RUST_HELPERS_H
> > +
> > +#include <linux/compiler_types.h>
> > +
> > +#ifdef __BINDGEN__
> > +#define __rust_helper
> > +#else
> > +#define __rust_helper inline
> > +#endif  
> 
> Could you mention this in the commit message? It's not obvious to me
> what this does and why it depends on __BINDGEN__ rather than
> CONFIG_RUST_INLINE_HELPERS.

I explained about the bindgen part in the new patch.

For CONFIG_RUST_INLINE_HELPERS, I don't think I have a good place to
fit it into the commit message, so I'll explain here:

`inline` in kernel is not the C `inline`. It's actually the GNU89
legacy inline, which both compiles as a standalone function (strong
external linkage) and provide inlining definition, so this works if
CONFIG_RUST_INLINE_HELPERS is not enabled.

Best,
Gary