Re: [PATCH 1/2] rust: auxiliary: add registration data to auxiliary devices

["Danilo Krummrich" <dakr@kernel.org>]

On Thu Apr 30, 2026 at 10:59 AM CEST, Alice Ryhl wrote:
> Is this really Rust-specific? Would you not want C drivers with the same
> pattern to do the same thing?

Unlike in C, it has the effect that it allows us to let the compiler ensure that
the initilization order in probe() is correct.

The only advantage I see in C is that it provides a dedicated place for drivers
to differentiate specific private data for when a driver registers multiple
auxiliary devices per instance.

>> +        // SAFETY: `ptr` is non-null and was set via `into_foreign()` in `Registration::new()`;
>> +        // `RegistrationData` is `#[repr(C)]` with `type_id` at offset 0, so reading a `TypeId`
>> +        // at the start of the allocation is valid regardless of `T`.
>> +        let type_id = unsafe { ptr.cast::<TypeId>().read() };
>> +        if type_id != TypeId::of::<T>() {
>> +            return Err(EINVAL);
>> +        }
>
> Right, okay, so if you put C stuff there, we need the layout to be
> compatible with Rust type ids.

Correct.

> Still, we could have Rust expose a couple methods to allow C code to use
> the same field with a null type id.

I think there are multiple options, e.g. two registration data pointers, one
common pointer and a boolean, one common pointer, plus a type id pointer, etc.

> But I guess this is all future work.

Yes, I rather do that once we actually need it.

>> +        let data = KBox::pin_init::<Error>(
>> +            try_pin_init!(RegistrationData {
>> +                type_id: TypeId::of::<T>(),
>> +                data <- data,
>> +            }),
>> +            GFP_KERNEL,
>> +        )?;
>> +
>> +        let boxed = KBox::new(Opaque::<bindings::auxiliary_device>::zeroed(), GFP_KERNEL)?;
>
> Use __GFP_ZERO here instead?

This is just moving existing code, but I guess we could add something like:

	pub fn zeroed(flags: Flags) -> Result<Self, AllocError>
	where
	    T: Zeroable,
	{
	    // SAFETY: `__GFP_ZERO` guarantees the memory is zeroed, and `T: Zeroable` guarantees
	    // that all-zeroes is a valid bit pattern for `T`.
	    Ok(unsafe { Self::new_uninit(flags | __GFP_ZERO)?.assume_init() })
	}

>> +        // SAFETY:
>> +        // - `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, which
>> +        //   has been initialized,
>> +        // - `modname.as_char_ptr()` is a NULL terminated string.
>> +        let ret = unsafe { bindings::__auxiliary_device_add(adev, modname.as_char_ptr()) };
>> +        if ret != 0 {
>> +            // SAFETY: `registration_data` was set above via `into_foreign()`.
>> +            let _ = unsafe {
>> +                Pin::<KBox<RegistrationData<T>>>::from_foreign((*adev).registration_data_rust)
>> +            };
>
> Nit: Please use `drop(unsafe { ... })` to explicitly drop.

Same here, just moving existing code, but I think it's fine to change while at
it.