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 (depending 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@alistair23.me>
---
Documentation/rust/general-information.rst | 10 +++---
rust/.gitignore | 2 ++
rust/Makefile | 37 ++++++++++++++++++++--
rust/bindgen_static_functions | 6 ++++
rust/bindings/lib.rs | 4 +++
rust/exports.c | 1 +
6 files changed, 53 insertions(+), 7 deletions(-)
create mode 100644 rust/bindgen_static_functions
diff --git a/Documentation/rust/general-information.rst b/Documentation/rust/general-information.rst
index 6146b49b6a98..632a60703c96 100644
--- a/Documentation/rust/general-information.rst
+++ b/Documentation/rust/general-information.rst
@@ -119,10 +119,12 @@ By including a C header from ``include/`` into
bindings for the included subsystem. After building, see the ``*_generated.rs``
output files in the ``rust/bindings/`` directory.
-For parts of the C header that ``bindgen`` does not auto generate, e.g. C
-``inline`` functions or non-trivial macros, it is acceptable to add a small
-wrapper function to ``rust/helpers/`` to make it available for the Rust side as
-well.
+C ``inline`` functions will only be generated if the function name is
+specified in ``rust/bindgen_static_functions``.
+
+For parts of the C header that ``bindgen`` does not auto generate, e.g.
+non-trivial macros, it is acceptable to add a small wrapper function
+to ``rust/helpers/`` to make it available for the Rust side as well.
Abstractions
~~~~~~~~~~~~
diff --git a/rust/.gitignore b/rust/.gitignore
index d3829ffab80b..741a18023801 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 6daaa4dc21db..431edc73e893 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -10,14 +10,17 @@ 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) += bindings.o kernel.o
+always-$(CONFIG_RUST) += bindings/bindings_generated.rs bindings/bindings_helpers_generated.rs \
+ bindings/bindings_generated_static.rs
+obj-$(CONFIG_RUST) += bindings.o kernel.o extern.o
always-$(CONFIG_RUST) += exports_helpers_generated.h \
- exports_bindings_generated.h exports_kernel_generated.h
+ exports_bindings_generated.h exports_bindings_static_generated.h \
+ exports_kernel_generated.h
always-$(CONFIG_RUST) += uapi/uapi_generated.rs
obj-$(CONFIG_RUST) += uapi.o
@@ -274,6 +277,21 @@ 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 \
+ --experimental --wrap-static-fns \
+ --wrap-static-fns-path $(src)/extern.c \
+ $(bindgen_static_functions) \
+ -o $@ -- $(bindgen_c_flags_final) -DMODULE \
+ $(bindgen_target_cflags) $(bindgen_target_extra)
+
+$(src)/extern.c: $(obj)/bindings/bindings_generated_static.rs \
+ $(src)/bindings/bindings_helper.h
+ @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 = ; \
@@ -282,6 +300,15 @@ $(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: private bindgen_static_functions = \
+ $(shell grep -Ev '^#|^$$' $(src)/bindgen_static_functions)
+$(obj)/bindings/bindings_generated_static.rs: $(src)/bindings/bindings_helper.h \
+ $(src)/bindgen_static_functions \
+ $(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 \
@@ -325,6 +352,9 @@ $(obj)/exports_helpers_generated.h: $(obj)/helpers/helpers.o FORCE
$(obj)/exports_bindings_generated.h: $(obj)/bindings.o FORCE
$(call if_changed,exports)
+$(obj)/exports_bindings_static_generated.h: $(obj)/extern.o FORCE
+ $(call if_changed,exports)
+
$(obj)/exports_kernel_generated.h: $(obj)/kernel.o FORCE
$(call if_changed,exports)
@@ -400,6 +430,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/bindgen_static_functions b/rust/bindgen_static_functions
new file mode 100644
index 000000000000..1f24360daeb3
--- /dev/null
+++ b/rust/bindgen_static_functions
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+
+# Don't generate structs
+--blocklist-type '.*'
+
+--allowlist-function blk_mq_rq_to_pdu
diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
index d6da3011281a..c991bcf607ec 100644
--- a/rust/bindings/lib.rs
+++ b/rust/bindings/lib.rs
@@ -34,6 +34,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,
diff --git a/rust/exports.c b/rust/exports.c
index 587f0e776aba..288958d2ebea 100644
--- a/rust/exports.c
+++ b/rust/exports.c
@@ -18,6 +18,7 @@
#include "exports_core_generated.h"
#include "exports_helpers_generated.h"
#include "exports_bindings_generated.h"
+#include "exports_bindings_static_generated.h"
#include "exports_kernel_generated.h"
// For modules using `rust/build_error.rs`.
--
2.47.0
"Alistair Francis" <alistair@alistair23.me> writes: <cut> > diff --git a/rust/exports.c b/rust/exports.c > index 587f0e776aba..288958d2ebea 100644 > --- a/rust/exports.c > +++ b/rust/exports.c > @@ -18,6 +18,7 @@ > #include "exports_core_generated.h" > #include "exports_helpers_generated.h" > #include "exports_bindings_generated.h" > +#include "exports_bindings_static_generated.h" Generating `exports_bindings_static_generated.h` depends on `exports.o`, which depends on `exports.c`. Does this not create chicken-egg kind of problem? Best regards, Andreas Hindborg
On Mon, Nov 11, 2024 at 10:07 PM Andreas Hindborg <a.hindborg@kernel.org> wrote: > > "Alistair Francis" <alistair@alistair23.me> writes: > > <cut> > > > diff --git a/rust/exports.c b/rust/exports.c > > index 587f0e776aba..288958d2ebea 100644 > > --- a/rust/exports.c > > +++ b/rust/exports.c > > @@ -18,6 +18,7 @@ > > #include "exports_core_generated.h" > > #include "exports_helpers_generated.h" > > #include "exports_bindings_generated.h" > > +#include "exports_bindings_static_generated.h" > > Generating `exports_bindings_static_generated.h` depends on `exports.o`, > which depends on `exports.c`. Does this not create chicken-egg kind of > problem? It is a bit confusing as there are a few levels of autogeneration, but Make happily handles it. `exports.c` depends on `exports_bindings_static_generated.h` But `exports_bindings_static_generated.h` depends on `extern.o` (extern not exports). `extern.o` then depends on `extern.c` `extern.c` then depends on `bindings_generated_static.rs`, which is generated by bindgen. So there isn't a chick-egg problem and this happily builds from a clean tree. Alistair > > Best regards, > Andreas Hindborg > >
"Alistair Francis" <alistair23@gmail.com> writes: > On Mon, Nov 11, 2024 at 10:07 PM Andreas Hindborg <a.hindborg@kernel.org> wrote: >> >> "Alistair Francis" <alistair@alistair23.me> writes: >> >> <cut> >> >> > diff --git a/rust/exports.c b/rust/exports.c >> > index 587f0e776aba..288958d2ebea 100644 >> > --- a/rust/exports.c >> > +++ b/rust/exports.c >> > @@ -18,6 +18,7 @@ >> > #include "exports_core_generated.h" >> > #include "exports_helpers_generated.h" >> > #include "exports_bindings_generated.h" >> > +#include "exports_bindings_static_generated.h" >> >> Generating `exports_bindings_static_generated.h` depends on `exports.o`, >> which depends on `exports.c`. Does this not create chicken-egg kind of >> problem? > > It is a bit confusing as there are a few levels of autogeneration, but > Make happily handles it. > > `exports.c` depends on `exports_bindings_static_generated.h` > > But `exports_bindings_static_generated.h` depends on `extern.o` > (extern not exports). > > `extern.o` then depends on `extern.c` > > `extern.c` then depends on `bindings_generated_static.rs`, which is > generated by bindgen. > > So there isn't a chick-egg problem and this happily builds from a clean tree. Right, I think I mixed up exports/extern. Anyway, it does not build for me. I applied it on top of `rust-next` and I get: .. │ CC rust/extern.o │ │/home/aeh/src/linux-rust/helpers/rust/extern.c:1:10: fatal error: '/home/aeh/src/linux-rust/helpers/bindings/bindings_helper.h' file not found │ │ 1 | #include "/home/aeh/src/linux-rust/helpers/bindings/bindings_helper.h" │ │ | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ │ │1 error generated. │ I am doing out of tree build - maybe that is the culprit? Best regards, Andreas Hindborg
On Tue, Nov 12, 2024 at 7:58 PM Andreas Hindborg <a.hindborg@kernel.org> wrote: > > "Alistair Francis" <alistair23@gmail.com> writes: > > > On Mon, Nov 11, 2024 at 10:07 PM Andreas Hindborg <a.hindborg@kernel.org> wrote: > >> > >> "Alistair Francis" <alistair@alistair23.me> writes: > >> > >> <cut> > >> > >> > diff --git a/rust/exports.c b/rust/exports.c > >> > index 587f0e776aba..288958d2ebea 100644 > >> > --- a/rust/exports.c > >> > +++ b/rust/exports.c > >> > @@ -18,6 +18,7 @@ > >> > #include "exports_core_generated.h" > >> > #include "exports_helpers_generated.h" > >> > #include "exports_bindings_generated.h" > >> > +#include "exports_bindings_static_generated.h" > >> > >> Generating `exports_bindings_static_generated.h` depends on `exports.o`, > >> which depends on `exports.c`. Does this not create chicken-egg kind of > >> problem? > > > > It is a bit confusing as there are a few levels of autogeneration, but > > Make happily handles it. > > > > `exports.c` depends on `exports_bindings_static_generated.h` > > > > But `exports_bindings_static_generated.h` depends on `extern.o` > > (extern not exports). > > > > `extern.o` then depends on `extern.c` > > > > `extern.c` then depends on `bindings_generated_static.rs`, which is > > generated by bindgen. > > > > So there isn't a chick-egg problem and this happily builds from a clean tree. > > Right, I think I mixed up exports/extern. > > Anyway, it does not build for me. I applied it on top of `rust-next` and > I get: > > .. > │ CC rust/extern.o │ > │/home/aeh/src/linux-rust/helpers/rust/extern.c:1:10: fatal error: '/home/aeh/src/linux-rust/helpers/bindings/bindings_helper.h' file not found │ > │ 1 | #include "/home/aeh/src/linux-rust/helpers/bindings/bindings_helper.h" │ > │ | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ │ > │1 error generated. │ Thanks for testing this The #include in `extern.c` is auto-generated by bindgen based on your current directory. So for an out of tree build I guess it's just wrong. I'll fixup the sed operation to ensure we correct the path to be a relative path instead. Alistair > > > I am doing out of tree build - maybe that is the culprit? > > > Best regards, > Andreas Hindborg > >
"Alistair Francis" <alistair@alistair23.me> writes:
> 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).
Very cool!
What does this mean for our minimum bindgen version? Is the feature safe
to use in our current required version of 0.65.1? Did the feature change
in significant ways between 0.65.1 and whatever version it will be
stable in?
> diff --git a/rust/bindgen_static_functions b/rust/bindgen_static_functions
> new file mode 100644
> index 000000000000..1f24360daeb3
> --- /dev/null
> +++ b/rust/bindgen_static_functions
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +# Don't generate structs
> +--blocklist-type '.*'
> +
> +--allowlist-function blk_mq_rq_to_pdu
Should this be moved to the "Remove blk helper" patch?
Best regards,
Andreas Hindborg
On Mon, Nov 11, 2024 at 9:47 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> "Alistair Francis" <alistair@alistair23.me> writes:
>
> > 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).
>
> Very cool!
>
> What does this mean for our minimum bindgen version? Is the feature safe
There are no changes to the minimum bindgen version :)
The next release of bindgen will support it without the experimental
flag [1], but 0.65.1 and newer will all work.
> to use in our current required version of 0.65.1? Did the feature change
> in significant ways between 0.65.1 and whatever version it will be
> stable in?
There are no breaking changes in the changelog that I can find. There
are improvements to support more functions, but as we use an
allow-list that doesn't affect us.
Eventually when the minimum bindgen is updated we can remove the
experimental flag, but otherwise it should work with everything newer
than 0.65.
1: https://github.com/rust-lang/rust-bindgen/blob/main/CHANGELOG.md?plain=1#L219
>
>
> > diff --git a/rust/bindgen_static_functions b/rust/bindgen_static_functions
> > new file mode 100644
> > index 000000000000..1f24360daeb3
> > --- /dev/null
> > +++ b/rust/bindgen_static_functions
> > @@ -0,0 +1,6 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +# Don't generate structs
> > +--blocklist-type '.*'
> > +
> > +--allowlist-function blk_mq_rq_to_pdu
>
> Should this be moved to the "Remove blk helper" patch?
I need a function here, otherwise nothing is generated and it fails to
build. As this would be added in the next patch anyway I just added it
here to maintain bisectability.
Alistair
>
>
> Best regards,
> Andreas Hindborg
>
>
© 2016 - 2024 Red Hat, Inc.