[PATCH 1/2] rust: add `pin-init` as a dependency to `bindings` and `uapi`

[PATCH 1/2] rust: add `pin-init` as a dependency to `bindings` and `uapi`

[Benno Lossin]

This allows `bindings` and `uapi` to implement `Zeroable` and use other
items from pin-init.

Signed-off-by: Benno Lossin <lossin@kernel.org>
---
 rust/Makefile | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/rust/Makefile b/rust/Makefile
index 3aca903a7d08..1bee11ad5452 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -513,17 +513,19 @@ $(obj)/ffi.o: private skip_gendwarfksyms = 1
 $(obj)/ffi.o: $(src)/ffi.rs $(obj)/compiler_builtins.o FORCE
 	+$(call if_changed_rule,rustc_library)
 
-$(obj)/bindings.o: private rustc_target_flags = --extern ffi
+$(obj)/bindings.o: private rustc_target_flags = --extern ffi --extern pin_init
 $(obj)/bindings.o: $(src)/bindings/lib.rs \
     $(obj)/ffi.o \
+    $(obj)/pin_init.o \
     $(obj)/bindings/bindings_generated.rs \
     $(obj)/bindings/bindings_helpers_generated.rs FORCE
 	+$(call if_changed_rule,rustc_library)
 
-$(obj)/uapi.o: private rustc_target_flags = --extern ffi
+$(obj)/uapi.o: private rustc_target_flags = --extern ffi --extern pin_init
 $(obj)/uapi.o: private skip_gendwarfksyms = 1
 $(obj)/uapi.o: $(src)/uapi/lib.rs \
     $(obj)/ffi.o \
+    $(obj)/pin_init.o \
     $(obj)/uapi/uapi_generated.rs FORCE
 	+$(call if_changed_rule,rustc_library)
 

base-commit: 22c3335c5dcd33063fe1894676a3a6ff1008d506
-- 
2.49.0

[PATCH 2/2] rust: derive `Zeroable` for all structs & unions generated by bindgen where possible

[Benno Lossin]

Using the `--with-derive-custom-{struct,union}` option of bindgen, add
`#[derive(MaybeZeroable)]` to every struct & union. This makes those
types implement `Zeroable` if all their fields implement it.

Sadly bindgen doesn't add custom derives to the `__BindgenBitfieldUnit`
struct. So manually implement `Zeroable` for that.

Signed-off-by: Benno Lossin <lossin@kernel.org>
---

This came from a discussion at [1]. The relevant parts for pin-init
already got merged into rust-next for v6.16, so we only need to enable
them for bindgen.

I'm not sure on the impact of build times and rust-analyzer. We're
adding a derive macro to every struct and union emitted by bindgen.
Building using my test-config took 28.6s before and 29.1s after this
change, but those are only two runs.

Maybe we have to reevaluate this when more C code is scanned by bindgen.

[1]: https://rust-for-linux.zulipchat.com/#narrow/channel/291565-Help/topic/Zeroable.20trait.20for.20C.20structs/with/509711564

---
 rust/bindgen_parameters | 4 ++++
 rust/bindings/lib.rs    | 9 +++++++++
 2 files changed, 13 insertions(+)

diff --git a/rust/bindgen_parameters b/rust/bindgen_parameters
index 0f96af8b9a7f..fa4c61ba028f 100644
--- a/rust/bindgen_parameters
+++ b/rust/bindgen_parameters
@@ -34,3 +34,7 @@
 # We use const helpers to aid bindgen, to avoid conflicts when constants are
 # recognized, block generation of the non-helper constants.
 --blocklist-item ARCH_SLAB_MINALIGN
+
+# Structs should implement Zeroable when all of their fields do.
+--with-derive-custom-struct .*=pin_init::MaybeZeroable
+--with-derive-custom-union .*=pin_init::MaybeZeroable
diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
index a08eb5518cac..38615c5b090d 100644
--- a/rust/bindings/lib.rs
+++ b/rust/bindings/lib.rs
@@ -33,6 +33,15 @@ mod bindings_raw {
     type __kernel_ssize_t = isize;
     type __kernel_ptrdiff_t = isize;
 
+    // `bindgen` doesn't automatically do this, see
+    // <https://github.com/rust-lang/rust-bindgen/issues/3196>
+    //
+    // SAFETY: `__BindgenBitfieldUnit<Storage>` is a newtype around `Storage`.
+    unsafe impl<Storage> pin_init::Zeroable for __BindgenBitfieldUnit<Storage> where
+        Storage: pin_init::Zeroable
+    {
+    }
+
     // Use glob import here to expose all helpers.
     // Symbols defined within the module will take precedence to the glob import.
     pub use super::bindings_helper::*;
-- 
2.49.0

Re: [PATCH 1/2] rust: add `pin-init` as a dependency to `bindings` and `uapi`

[Tamir Duberstein]

On Tue, May 20, 2025 at 3:23 PM Benno Lossin <lossin@kernel.org> wrote:
>
> This allows `bindings` and `uapi` to implement `Zeroable` and use other
> items from pin-init.
>
> Signed-off-by: Benno Lossin <lossin@kernel.org>

Please remember to update `generate_rust_analyzer.py`.

Re: [PATCH 1/2] rust: add `pin-init` as a dependency to `bindings` and `uapi`

[Benno Lossin]

On Tue May 20, 2025 at 9:23 PM CEST, Benno Lossin wrote:
> This allows `bindings` and `uapi` to implement `Zeroable` and use other
> items from pin-init.
>
> Signed-off-by: Benno Lossin <lossin@kernel.org>
> ---
>  rust/Makefile | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

I also forgot to add this:

Co-developed-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://rust-for-linux.zulipchat.com/#narrow/channel/291565-Help/topic/Zeroable.20trait.20for.20C.20structs/near/510264158

Will add it with the new version that also fixes rust-analyzer.

---
Cheers,
Benno

Re: [PATCH 2/2] rust: derive `Zeroable` for all structs & unions generated by bindgen where possible

[Alice Ryhl]

On Tue, May 20, 2025 at 12:23 PM Benno Lossin <lossin@kernel.org> wrote:
>
> Using the `--with-derive-custom-{struct,union}` option of bindgen, add
> `#[derive(MaybeZeroable)]` to every struct & union. This makes those
> types implement `Zeroable` if all their fields implement it.

I think this is a great idea.

> Sadly bindgen doesn't add custom derives to the `__BindgenBitfieldUnit`
> struct. So manually implement `Zeroable` for that.
>
> Signed-off-by: Benno Lossin <lossin@kernel.org>
> ---
>
> This came from a discussion at [1]. The relevant parts for pin-init
> already got merged into rust-next for v6.16, so we only need to enable
> them for bindgen.
>
> I'm not sure on the impact of build times and rust-analyzer. We're
> adding a derive macro to every struct and union emitted by bindgen.
> Building using my test-config took 28.6s before and 29.1s after this
> change, but those are only two runs.
>
> Maybe we have to reevaluate this when more C code is scanned by bindgen.
>
> [1]: https://rust-for-linux.zulipchat.com/#narrow/channel/291565-Help/topic/Zeroable.20trait.20for.20C.20structs/with/509711564
>
> ---
>  rust/bindgen_parameters | 4 ++++
>  rust/bindings/lib.rs    | 9 +++++++++
>  2 files changed, 13 insertions(+)
>
> diff --git a/rust/bindgen_parameters b/rust/bindgen_parameters
> index 0f96af8b9a7f..fa4c61ba028f 100644
> --- a/rust/bindgen_parameters
> +++ b/rust/bindgen_parameters
> @@ -34,3 +34,7 @@
>  # We use const helpers to aid bindgen, to avoid conflicts when constants are
>  # recognized, block generation of the non-helper constants.
>  --blocklist-item ARCH_SLAB_MINALIGN
> +
> +# Structs should implement Zeroable when all of their fields do.
> +--with-derive-custom-struct .*=pin_init::MaybeZeroable
> +--with-derive-custom-union .*=pin_init::MaybeZeroable

Maybe add an import in bindings/lib.rs and use it directly? Seems a
bit nicer to read without the prefix.

> diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
> index a08eb5518cac..38615c5b090d 100644
> --- a/rust/bindings/lib.rs
> +++ b/rust/bindings/lib.rs
> @@ -33,6 +33,15 @@ mod bindings_raw {
>      type __kernel_ssize_t = isize;
>      type __kernel_ptrdiff_t = isize;
>
> +    // `bindgen` doesn't automatically do this, see
> +    // <https://github.com/rust-lang/rust-bindgen/issues/3196>
> +    //
> +    // SAFETY: `__BindgenBitfieldUnit<Storage>` is a newtype around `Storage`.
> +    unsafe impl<Storage> pin_init::Zeroable for __BindgenBitfieldUnit<Storage> where
> +        Storage: pin_init::Zeroable
> +    {
> +    }
> +
>      // Use glob import here to expose all helpers.
>      // Symbols defined within the module will take precedence to the glob import.
>      pub use super::bindings_helper::*;
> --
> 2.49.0
>

Re: [PATCH 2/2] rust: derive `Zeroable` for all structs & unions generated by bindgen where possible

[Benno Lossin]

On Tue May 20, 2025 at 11:38 PM CEST, Alice Ryhl wrote:
> On Tue, May 20, 2025 at 12:23 PM Benno Lossin <lossin@kernel.org> wrote:
>> diff --git a/rust/bindgen_parameters b/rust/bindgen_parameters
>> index 0f96af8b9a7f..fa4c61ba028f 100644
>> --- a/rust/bindgen_parameters
>> +++ b/rust/bindgen_parameters
>> @@ -34,3 +34,7 @@
>>  # We use const helpers to aid bindgen, to avoid conflicts when constants are
>>  # recognized, block generation of the non-helper constants.
>>  --blocklist-item ARCH_SLAB_MINALIGN
>> +
>> +# Structs should implement Zeroable when all of their fields do.
>> +--with-derive-custom-struct .*=pin_init::MaybeZeroable
>> +--with-derive-custom-union .*=pin_init::MaybeZeroable
>
> Maybe add an import in bindings/lib.rs and use it directly? Seems a
> bit nicer to read without the prefix.

I don't mind doing this, but I don't really see the value in it. I view
the generated bindings from time to time, but having the extra path
wouldn't bother me.

I'll add it to the changes on the next version, but if you have some
better justification, I'd love to hear it.

---
Cheers,
Benno