[PATCH] rust: bindings: Support some inline static functions

[PATCH] rust: bindings: Support some inline static functions

[Alistair Francis]

The kernel includes a large number of static inline functions that are
defined in header files. One example is the crypto_shash_descsize()
function which is defined in hash.h as

static inline unsigned int crypto_shash_descsize(struct crypto_shash *tfm)
{
	return tfm->descsize;
}

bindgen is currently unable to generate bindings to these functions as
they are not publically exposed (they are static after all).

The Rust code currently uses rust_helper_* functions, such as
rust_helper_alloc_pages() for example to call the static inline
functions. But this is a hassle as someone needs to write a C helper
function.

Instead we can use the bindgen wrap-static-fns feature. The feature
is marked as experimental, but has recently been promoted to
non-experimental (dependig on your version of bindgen).

By supporting wrap-static-fns we automatically generate a C file called
extern.c that exposes the static inline functions, for example like this

```
unsigned int crypto_shash_descsize__extern(struct crypto_shash *tfm) { return crypto_shash_descsize(tfm); }
```

The nice part is that this is auto-generated.

We then also get a bindings_generate_static.rs file with the Rust
binding, like this

```
extern "C" {
    #[link_name = "crypto_shash_descsize__extern"]
    pub fn crypto_shash_descsize(tfm: *mut crypto_shash) -> core::ffi::c_uint;
}
```

So now we can use the static inline functions just like normal
functions.

There are a bunch of static inline functions that don't work though, because
the C compiler fails to build extern.c:
 * functions with inline asm generate "operand probably does not match constraints"
   errors (rip_rel_ptr() for example)
 * functions with bit masks (u32_encode_bits() and friends) result in
   "call to ‘__bad_mask’ declared with attribute error: bad bitfield mask"
   errors

As well as that any static inline function that calls a function that has been
kconfig-ed out will fail to link as the function being called isn't built
(mdio45_ethtool_gset_npage for example)

Due to these failures we use a allow-list system (where functions must
be manually enabled).

Link: https://github.com/rust-lang/rust-bindgen/discussions/2405
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
If this is accepted or at least Acked I can work on removing the
existing rust_helper_* functions and convert them to be auto-generated.

 rust/.gitignore      |  2 ++
 rust/Makefile        | 29 +++++++++++++++++++++++++++--
 rust/bindings/lib.rs |  4 ++++
 3 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/rust/.gitignore b/rust/.gitignore
index d3829ffab80ba..741a180238013 100644
--- a/rust/.gitignore
+++ b/rust/.gitignore
@@ -1,10 +1,12 @@
 # SPDX-License-Identifier: GPL-2.0
 
 bindings_generated.rs
+bindings_generated_static.rs
 bindings_helpers_generated.rs
 doctests_kernel_generated.rs
 doctests_kernel_generated_kunit.c
 uapi_generated.rs
 exports_*_generated.h
+extern.c
 doc/
 test/
diff --git a/rust/Makefile b/rust/Makefile
index b5e0a73b78f3e..649346cfc373e 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -10,12 +10,13 @@ always-$(CONFIG_RUST) += exports_core_generated.h
 # 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
+CFLAGS_REMOVE_extern.o = -Wmissing-prototypes -Wmissing-declarations -Wdiscarded-qualifiers
 
 always-$(CONFIG_RUST) += libmacros.so
 no-clean-files += libmacros.so
 
-always-$(CONFIG_RUST) += bindings/bindings_generated.rs bindings/bindings_helpers_generated.rs
-obj-$(CONFIG_RUST) += alloc.o bindings.o kernel.o
+always-$(CONFIG_RUST) += bindings/bindings_generated.rs bindings/bindings_generated_static.rs bindings/bindings_helpers_generated.rs
+obj-$(CONFIG_RUST) += alloc.o bindings.o kernel.o extern.o
 always-$(CONFIG_RUST) += exports_alloc_generated.h exports_helpers_generated.h \
     exports_bindings_generated.h exports_kernel_generated.h
 
@@ -267,6 +268,10 @@ endif
 
 bindgen_c_flags_final = $(bindgen_c_flags_lto) -D__BINDGEN__
 
+bindgen_static_functions = --experimental --wrap-static-fns \
+	--blocklist-type '.*' \
+	--allowlist-function crypto_shash_descsize
+
 quiet_cmd_bindgen = BINDGEN $@
       cmd_bindgen = \
 	$(BINDGEN) $< $(bindgen_target_flags) \
@@ -275,6 +280,19 @@ quiet_cmd_bindgen = BINDGEN $@
 		-o $@ -- $(bindgen_c_flags_final) -DMODULE \
 		$(bindgen_target_cflags) $(bindgen_target_extra)
 
+quiet_cmd_bindgen_static = BINDGEN $@
+      cmd_bindgen_static = \
+	$(BINDGEN) $< $(bindgen_target_flags) \
+		--use-core --with-derive-default --ctypes-prefix core::ffi --no-layout-tests \
+		--no-debug '.*' --enable-function-attribute-detection \
+		$(bindgen_static_functions) \
+		-o $@ -- $(bindgen_c_flags_final) -DMODULE \
+		$(bindgen_target_cflags) $(bindgen_target_extra)
+
+$(src)/extern.c: $(obj)/bindings/bindings_generated_static.rs
+	@cp /tmp/bindgen/extern.c $(src)/
+	@sed -i 's|rust/bindings|bindings|g' $@
+
 $(obj)/bindings/bindings_generated.rs: private bindgen_target_flags = \
     $(shell grep -Ev '^#|^$$' $(src)/bindgen_parameters)
 $(obj)/bindings/bindings_generated.rs: private bindgen_target_extra = ; \
@@ -283,6 +301,12 @@ $(obj)/bindings/bindings_generated.rs: $(src)/bindings/bindings_helper.h \
     $(src)/bindgen_parameters FORCE
 	$(call if_changed_dep,bindgen)
 
+$(obj)/bindings/bindings_generated_static.rs: private bindgen_target_flags = \
+    $(shell grep -Ev '^#|^$$' $(src)/bindgen_parameters)
+$(obj)/bindings/bindings_generated_static.rs: $(src)/bindings/bindings_helper.h \
+    $(src)/bindgen_parameters FORCE
+	$(call if_changed_dep,bindgen_static)
+
 $(obj)/uapi/uapi_generated.rs: private bindgen_target_flags = \
     $(shell grep -Ev '^#|^$$' $(src)/bindgen_parameters)
 $(obj)/uapi/uapi_generated.rs: $(src)/uapi/uapi_helper.h \
@@ -410,6 +434,7 @@ $(obj)/build_error.o: $(src)/build_error.rs $(obj)/compiler_builtins.o FORCE
 $(obj)/bindings.o: $(src)/bindings/lib.rs \
     $(obj)/compiler_builtins.o \
     $(obj)/bindings/bindings_generated.rs \
+    $(obj)/bindings/bindings_generated_static.rs \
     $(obj)/bindings/bindings_helpers_generated.rs FORCE
 	+$(call if_changed_rule,rustc_library)
 
diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
index 93a1a3fc97bc9..63b915a8e4fbf 100644
--- a/rust/bindings/lib.rs
+++ b/rust/bindings/lib.rs
@@ -33,6 +33,10 @@ mod bindings_raw {
         env!("OBJTREE"),
         "/rust/bindings/bindings_generated.rs"
     ));
+    include!(concat!(
+        env!("OBJTREE"),
+        "/rust/bindings/bindings_generated_static.rs"
+    ));
 }
 
 // When both a directly exposed symbol and a helper exists for the same function,
-- 
2.47.0

Re: [PATCH] rust: bindings: Support some inline static functions

[Miguel Ojeda]

On Tue, Nov 5, 2024 at 3:22 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> If this is accepted or at least Acked I can work on removing the
> existing rust_helper_* functions and convert them to be auto-generated.

Yeah, we have tracked the feature for a long time, please see:

    https://github.com/Rust-for-Linux/linux/issues/353

So it is definitely in our plan to use (assuming it works).

But, yeah, we would need the patch with the conversions, to show how
it would look in practice (i.e. comparing before/after etc.), i.e. the
usual kernel rule.

> +       --allowlist-function crypto_shash_descsize

We probably should put this into something similar to
`rust/bindgen_parameters` (and remove this variable).

> +       @cp /tmp/bindgen/extern.c $(src)/

I don't think we can use/assume `/tmp` -- we will need
`--wrap-static-fns-path` or similar.

Thanks for trying out the new feature!

Cheers,
Miguel

Re: [PATCH] rust: bindings: Support some inline static functions

[kernel test robot]

Hi Alistair,

kernel test robot noticed the following build errors:

[auto build test ERROR on v6.12-rc6]
[also build test ERROR on linus/master]
[cannot apply to rust/rust-next next-20241105]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Alistair-Francis/rust-bindings-Support-some-inline-static-functions/20241105-102435
base:   v6.12-rc6
patch link:    https://lore.kernel.org/r/20241105022143.1087112-1-alistair.francis%40wdc.com
patch subject: [PATCH] rust: bindings: Support some inline static functions
config: um-randconfig-001-20241106 (https://download.01.org/0day-ci/archive/20241106/202411060829.3NoGuTYU-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 592c0fe55f6d9a811028b5f3507be91458ab2713)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241106/202411060829.3NoGuTYU-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411060829.3NoGuTYU-lkp@intel.com/

All errors (new ones prefixed by >>):

>> cp: cannot stat '/tmp/bindgen/extern.c': No such file or directory

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] rust: bindings: Support some inline static functions

[Alistair Francis]

On Tue, Nov 5, 2024 at 7:32 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Tue, Nov 5, 2024 at 3:22 AM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > If this is accepted or at least Acked I can work on removing the
> > existing rust_helper_* functions and convert them to be auto-generated.
>
> Yeah, we have tracked the feature for a long time, please see:
>
>     https://github.com/Rust-for-Linux/linux/issues/353
>
> So it is definitely in our plan to use (assuming it works).
>
> But, yeah, we would need the patch with the conversions, to show how
> it would look in practice (i.e. comparing before/after etc.), i.e. the
> usual kernel rule.

Great! I have done the conversions in a patch series here:
https://lore.kernel.org/lkml/20241107020831.1561063-1-alistair.francis@wdc.com/T/#t

>
> > +       --allowlist-function crypto_shash_descsize
>
> We probably should put this into something similar to
> `rust/bindgen_parameters` (and remove this variable).

Fixed in https://lore.kernel.org/lkml/20241107020831.1561063-1-alistair.francis@wdc.com/T/#t

>
> > +       @cp /tmp/bindgen/extern.c $(src)/
>
> I don't think we can use/assume `/tmp` -- we will need
> `--wrap-static-fns-path` or similar.

Fixed in https://lore.kernel.org/lkml/20241107020831.1561063-1-alistair.francis@wdc.com/T/#t

Alistair

>
> Thanks for trying out the new feature!
>
> Cheers,
> Miguel